Skip to content

gh-128661: Remove DeprecationWarning in evaluate_forward_ref #128930

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

Merged
merged 4 commits into from
Apr 3, 2025
Merged

gh-128661: Remove DeprecationWarning in evaluate_forward_ref #128930

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc68f2e
gh-128661: Remove DeprecationWarning in evaluate_forward_ref
JelleZijlstra Jan 17, 2025
47c2193
Merge branch 'main' into efr
JelleZijlstra Mar 3, 2025
9f69ffe
Better docs for fwdref evaluation
JelleZijlstra Mar 3, 2025
ca02c4a
Merge branch 'main' into efr
JelleZijlstra Apr 1, 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
5 changes: 4 additions & 1 deletion Doc/library/annotationlib.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,10 @@ Classes
the global and local namespaces in which the name is evaluated.
*type_params*, if given, must be a tuple of
:ref:`type parameters <type-params>` that are in scope while the forward
reference is being evaluated. *owner* is the object that owns the
reference is being evaluated. This parameter should be provided (though it
may be an empty tuple) if the *owner* parameter is not given and the
:class:`~ForwardRef` instance does not have an owner set.
*owner* is the object that owns the
annotation from which the forward reference derives, usually a function,
class, or module.

Expand Down
2 changes: 1 addition & 1 deletion Doc/library/typing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3439,7 +3439,7 @@ Introspection helpers
the global and local namespaces.
*type_params* is a tuple of :ref:`type parameters <type-params>` that
are in scope when evaluating the forward reference.
This parameter must be provided (though it may be an empty tuple) if *owner*
This parameter should be provided (though it may be an empty tuple) if *owner*
is not given and the forward reference does not already have an owner set.
*format* specifies the format of the annotation and is a member of
the :class:`annotationlib.Format` enum.
Expand Down
15 changes: 1 addition & 14 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7284,20 +7284,7 @@ def test_evaluate_forward_ref(self):

def test_evaluate_forward_ref_no_type_params(self):
ref = ForwardRef('int')
with self.assertWarnsRegex(
DeprecationWarning,
(
"Failing to pass a value to the 'type_params' parameter "
"of 'typing.evaluate_forward_ref' is deprecated, "
"as it leads to incorrect behaviour"
),
):
typing.evaluate_forward_ref(ref)

# No warnings when `type_params` is passed:
with warnings.catch_warnings(record=True) as w:
typing.evaluate_forward_ref(ref, type_params=())
self.assertEqual(w, [])
self.assertIs(typing.evaluate_forward_ref(ref), int)


class CollectionsAbcTests(BaseTestCase):
Expand Down
7 changes: 2 additions & 5 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ def evaluate_forward_ref(
owner=None,
globals=None,
locals=None,
type_params=_sentinel,
type_params=None,
Copy link
Member

@sobolevn sobolevn Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

My concern is that it would be rather easy to miss. This function has multiple parameters, __type_params__ is a new thing, which not many people are familiar with.

Proposed API makes it easy to miss.

And when missed, simple code like -> T and evaluate_forward_ref('T', format=VALUE) (instead of evaluate_forward_ref('T', format=VALUE, type_params=func.__type_params__)) can raise an error.

On the other hand, type_params is indeed optional in a sense, that users can't even provide type_params in all cases.

I have very mixed feelings about it :(

Copy link
Member Author

Choose a reason for hiding this comment

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

In your example the user should rather pass owner=. Pointing them to use type_params= only makes the API more complicated.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I am not yet fully familiar with this new API. Why do we even need type_params then, if it can be replaced with owner=? 🤔

format=annotationlib.Format.VALUE,
_recursive_guard=frozenset(),
):
Expand All @@ -1044,15 +1044,12 @@ def evaluate_forward_ref(
infer the namespaces to use for looking up names. *globals* and *locals*
can also be explicitly given to provide the global and local namespaces.
*type_params* is a tuple of type parameters that are in scope when
evaluating the forward reference. This parameter must be provided (though
evaluating the forward reference. This parameter should be provided (though
it may be an empty tuple) if *owner* is not given and the forward reference
does not already have an owner set. *format* specifies the format of the
annotation and is a member of the annotationlib.Format enum.

"""
if type_params is _sentinel:
_deprecation_warning_for_no_type_params_passed("typing.evaluate_forward_ref")
type_params = ()
if format == annotationlib.Format.STRING:
return forward_ref.__forward_arg__
if forward_ref.__forward_arg__ in _recursive_guard:
Expand Down
Loading