Skip to content

gh-130870 Fix _eval_type Handling for GenericAlias with Unflattened Arguments and Union Types #130897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+64 −1
Open

gh-130870 Fix _eval_type Handling for GenericAlias with Unflattened Arguments and Union Types #130897

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b38f689
Update typing.py
sharktide Mar 5, 2025
bea2a10
gh-130860 finalize pr changes
sharktide Mar 5, 2025
d6ea064
Merge branch 'python:main' into patch-130870
sharktide Mar 5, 2025
3af57b4
📜🤖 Added by blurb_it.
blurb-it[bot] Mar 5, 2025
d3a633c
Update 2025-03-05-21-48-22.gh-issue-130870.uDz6AQ.rst
sharktide Mar 5, 2025
b85d920
Update 2025-03-05-21-48-22.gh-issue-130870.uDz6AQ.rst
sharktide Mar 6, 2025
b0db6a6
Update 2025-03-05-21-48-22.gh-issue-130870.uDz6AQ.rst
sharktide Mar 6, 2025
596b6f5
gh-130897 Add test cases
sharktide Mar 6, 2025
37b760c
Update test_typing.py
sharktide Mar 6, 2025
aeb602b
Remove whitespace test_typing.py
sharktide Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ def _eval_type(t, globalns, localns, type_params=_sentinel, *, recursive_guard=f
if ev_args == t.__args__:
return t
if isinstance(t, GenericAlias):
return GenericAlias(t.__origin__, ev_args)
if _should_unflatten_callable_args(t, ev_args):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for calling _should_unflatten_callable_args here?

Please add tests demonstrating the new behavior.

Copy link
Contributor Author

@sharktide sharktide Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why call _should_unflatten_callable_args:

_should_unflatten_callable_args is used to make sure that the arguments of a callable type are handled properlt sometimes, the arguments are grouped together like in a list and need to be unpacked into separate arguments. I'll link a modified version of the test script here and then also link the results

*Sorry my capitalization sucks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I made a mistake. The appropriate example should be

Callable[[arg1, arg2], result]

Copy link
Contributor Author

@sharktide sharktide Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my test script:

import typing
from typing import ForwardRef, Union, List, Tuple, Dict, Callable, Any
import functools
import operator
from typing import _eval_type

class MyType:
    pass

# Test 1
def test_forward_ref():
    fwd_ref = ForwardRef('MyType')
    result = _eval_type(fwd_ref, globals(), locals())
    assert result is MyType, f"Expected MyType, got {result}"

# Test 2
def test_generic_alias():
    fwd_ref = ForwardRef('MyType')
    generic_list = List[fwd_ref]
    result = _eval_type(generic_list, globals(), locals())
    assert result == List[MyType], f"Expected List[MyType], got {result}"

# Test 3
def test_union():
    fwd_ref_1 = ForwardRef('MyType')
    fwd_ref_2 = ForwardRef('int')
    union_type = Union[fwd_ref_1, fwd_ref_2]
    
    result = _eval_type(union_type, globals(), locals())
    assert result == Union[MyType, int], f"Expected Union[MyType, int], got {result}"

# Test 4
def test_recursive_forward_ref():
    recursive_ref = ForwardRef('RecursiveType')
    globals()['RecursiveType'] = recursive_ref

    recursive_type = Dict[str, List[recursive_ref]]
    
    result = _eval_type(recursive_type, globals(), locals(), recursive_guard={recursive_ref})
    
    assert result == Dict[str, List[recursive_ref]], f"Expected Dict[str, List[RecursiveType]], got {result}"


# Test 5
def test_callable_unpacking():
    fwd_ref = ForwardRef('MyType')
    callable_type = Callable[[fwd_ref, int], str]
    result = _eval_type(callable_type, globals(), locals())
    
    assert result == Callable[[MyType, int], str], f"Expected Callable[[MyType, int], str], got {result}"

# Test 6
def test_unpacked_generic():
    fwd_ref = ForwardRef('MyType')
    generic_type = Tuple[fwd_ref, int]
    
    result = _eval_type(generic_type, globals(), locals())
    assert result == Tuple[MyType, int], f"Expected Tuple[MyType, int], got {result}"

# Test 7
def test_preservation_of_type():
    fwd_ref_1 = ForwardRef('MyType')
    fwd_ref_2 = ForwardRef('int')
    complex_type = Dict[str, Union[fwd_ref_1, fwd_ref_2]]
    
    result = _eval_type(complex_type, globals(), locals())
    assert result == Dict[str, Union[MyType, int]], f"Expected Dict[str, Union[MyType, int]], got {result}"

# Test 8 (As per request of @JelleZijlstra)
def test_callable_unflattening():
    callable_type = Callable[[int, str], bool]
    result = _eval_type(callable_type, globals(), locals(), type_params=())
    assert result == Callable[[int, str], bool], f"Expected Callable[[int, str], bool], got {result}"

    callable_type_packed = Callable[[int, str], bool]  # Correct format for callable
    result = _eval_type(callable_type_packed, globals(), locals(), type_params=())
    assert result == Callable[[int, str], bool], f"Expected Callable[[int, str], bool], got {result}"



def run_tests():
    test_forward_ref()
    test_generic_alias()
    test_union()
    test_recursive_forward_ref()
    test_callable_unpacking()
    test_unpacked_generic()
    test_preservation_of_type()
    test_callable_unflattening()
    print("All tests passed!")

# Run the tests
run_tests()

And result:

PS C:\Users\---\Documents\GitHub\cpython> .\PCbuild\amd64\python_d.exe test.py
C:\Users\---\Documents\GitHub\cpython\test.py:16: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(fwd_ref, globals(), locals())
C:\Users\---\Documents\GitHub\cpython\test.py:24: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(generic_list, globals(), locals())
C:\Users\---\Documents\GitHub\cpython\test.py:33: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(union_type, globals(), locals())
C:\Users\---\Documents\GitHub\cpython\test.py:47: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(recursive_type, globals(), locals(), recursive_guard={recursive_ref})
C:\Users\---\Documents\GitHub\cpython\test.py:57: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(callable_type, globals(), locals())
C:\Users\---\Documents\GitHub\cpython\test.py:66: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(generic_type, globals(), locals())
C:\Users\---\Documents\GitHub\cpython\test.py:76: DeprecationWarning: Failing to pass a value to the 'type_params' parameter of 'typing._eval_type' is deprecated, as it leads to incorrect behaviour when calling typing._eval_type on a stringified annotation that references a PEP 695 type parameter. It will be disallowed in Python 3.15.
  result = _eval_type(complex_type, globals(), locals())
All tests passed!

Note: The DeprecationWarning is a problem with my test script rather than the modified function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test cases should go in test_typing.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JelleZijlstra Done added class TestGenericAliasHandling to test_typing.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JelleZijlstra Ready for review

return t.__class__(t.__origin__, (ev_args[:-1], ev_args[-1]))
return t.__class__(t.__origin__, ev_args)
if isinstance(t, Union):
return functools.reduce(operator.or_, ev_args)
else:
Expand Down
6 changes: 6 additions & 0 deletions Misc/NEWS.d/next/Library/2025-03-05-21-48-22.gh-issue-130870.uDz6AQ.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Type: Bugfix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too much text; we don't need the "type" and "title" (we get that from the linked issue number). We don't have to mention that the tests pass: if they didn't, we wouldn't merge the change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is screaming AI-generated to me, as well as the PR description.

@sharktide, it's OK to use AI for some cases, but please refer to the devguide for the rules on using LLMs to generate PRs.

Copy link
Contributor Author

@sharktide sharktide Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was running out of time as I was at school and I had a short break so I asked chatgpt to make the news entry and pr desc. The first few lines of the news entry I basically copied from my previous prs as I don’t have much experience

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too much text; we don't need the "type" and "title" (we get that from the linked issue number). We don't have to mention that the tests pass: if they didn't, we wouldn't merge the change.

I’ll change it when I get a chance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, feel free to ask us for help or clarification on the process.

Copy link
Contributor Author

@sharktide sharktide Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to this

Issue: :gh:`130870`

Detailed changes:
This change improves the handling of ``GenericAlias`` and ``Union`` types in ``_eval_type``, ensuring that callable arguments for ``GenericAlias`` are unflattened and that ``Union`` types are properly evaluated.

Title: Fix ``_eval_type`` Handling for ``GenericAlias`` with Unflattened Arguments and ``Union`` Types
Issue: :gh:`130870`

Detailed changes:
This change improves the handling of ``GenericAlias`` and ``Union`` types in ``_eval_type``, ensuring that callable arguments for ``GenericAlias`` are unflattened and that ``Union`` types are properly evaluated. It resolves complex annotations, including recursive and generic types. All relevant tests, including those for forward references, generics, ``Union`` types, and recursion, passed successfully without any issues.
Loading