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-85668: Create public API for typing._eval_type #21753

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Dominik1123
Copy link
Contributor

@Dominik1123 Dominik1123 commented Aug 6, 2020

"""Evaluate all forward references in the given type t.
For use of globalns and localns see the docstring for get_type_hints().
"""
return _eval_type(t, globalns, localns)
Copy link
Member

Choose a reason for hiding this comment

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

Hm, you have to fill in globalns and localns if they are None, the same as get_type_hints does. And add tests for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should I fill them with? Unlike get_type_hints, eval_type is not aware of the context it's being used in, so it's the user's responsibility to provide one (in form of globalns and localns). get_type_hints retrieves the namespace from the passed object, but eval_type only receives a type (declaration).

Copy link
Member

Choose a reason for hiding this comment

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

Good point, but then you shouldn't give then a default value of None. :-)

My proposal: require globalns, and let localns default to globalns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention behind giving a default of None to both parameters was to not prioritize one over the other. If one should take priority, globalns is certainly the better choice already from considering the order of parameters (to be consistent with get_type_hints).

But do we even need a distinction between global and local namespace at this point? What about exposing a single parameter namespace without default? The user can always do something like eval_type(t, namespace=globals() | locals()).

By the way, I realized that ForwardRef._evaluate sets globalns = localns if the former is None and the latter isn't. These are then passed to eval. Since eval accepts any mapping for locals but requires a dict for globals this can lead to the following error:

>>> from types import MappingProxyType
>>> _eval_type(ForwardRef('int'), globalns=None, localns=MappingProxyType({}))
[...]
  File ".../lib/python3.8/typing.py", line 518, in _evaluate
    eval(self.__forward_code__, globalns, localns),
TypeError: globals must be a real dict; try eval(expr, {}, mapping)

Does that mean that either ForwardRef._evaluate or the new eval_type should include another safety check?

@bswck
Copy link

bswck commented May 9, 2023

It would have a use case for me. When is this going to be finished?

@gvanrossum
Copy link
Member

It would have a use case for me. When is this going to be finished?

Never, unless someone revives it.

@AlexWaygood AlexWaygood changed the title bpo-41496: Create public API for typing._eval_type gh-85668: Create public API for typing._eval_type Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants