Skip to content
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.

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

gh-128661: Remove DeprecationWarning in evaluate_forward_ref #128930

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 18 additions & 10 deletions Doc/library/annotationlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -172,30 +172,38 @@ Classes
:class:`~ForwardRef`. The string may not be exactly equivalent
to the original source.

.. method:: evaluate(*, globals=None, locals=None, type_params=None, owner=None)
.. method:: evaluate(*, owner=None, globals=None, locals=None, type_params=None)

Evaluate the forward reference, returning its value.

This may throw an exception, such as :exc:`NameError`, if the forward
reference refers to names that do not exist. The arguments to this
reference refers to a name that cannot be resolved. The arguments to this
method can be used to provide bindings for names that would otherwise
be undefined.

The *owner* parameter provides the preferred mechanism for passing scope
information to this method. The owner of a :class:`~ForwardRef` is the
object that contains the annotation from which the :class:`~ForwardRef`
derives, such as a module object, type object, or function object.

The *globals*, *locals*, and *type_params* parameters provide a more precise
mechanism for influencing the names that are available when the :class:`~ForwardRef`
is evaluated. *globals* and *locals* are passed to :func:`eval`, representing
the global and local namespaces in which the name is evaluated.
The *type_params* parameter is relevant for objects created using the native
syntax for :ref:`generic classes <generic-classes>` and :ref:`functions <generic-functions>`.
It is a tuple of :ref:`type parameters <type-params>` that are in scope
while the forward reference is being evaluated. For example, if evaluating a
:class:`~ForwardRef` retrieved from an annotation found in the class namespace
of a generic class ``C``, *type_params* should be set to ``C.__type_params__``.

:class:`~ForwardRef` instances returned by :func:`get_annotations`
retain references to information about the scope they originated from,
so calling this method with no further arguments may be sufficient to
evaluate such objects. :class:`~ForwardRef` instances created by other
means may not have any information about their scope, so passing
arguments to this method may be necessary to evaluate them successfully.

*globals* and *locals* are passed to :func:`eval`, representing
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
annotation from which the forward reference derives, usually a function,
class, or module.

.. important::

Once a :class:`~ForwardRef` instance has been evaluated, it caches
Expand Down
12 changes: 2 additions & 10 deletions Doc/library/typing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3431,16 +3431,8 @@ Introspection helpers
* Supports the :attr:`~annotationlib.Format.FORWARDREF` and
:attr:`~annotationlib.Format.STRING` formats.

*forward_ref* must be an instance of :class:`~annotationlib.ForwardRef`.
*owner*, if given, should be the object that holds the annotations that
the forward reference derived from, such as a module, class object, or function.
It is used to 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 :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*
is not given and the forward reference does not already have an owner set.
See the documentation for :meth:`annotationlib.ForwardRef.evaluate` for
the meaning of the *owner*, *globals*, *locals*, and *type_params* parameters.
*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
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
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

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