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

typing.get_type_hints with TYPE_CHECKING imports / getting hints for single argument #87629

Open
The-Compiler mannequin opened this issue Mar 10, 2021 · 15 comments
Open
Labels
3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@The-Compiler
Copy link
Mannequin

The-Compiler mannequin commented Mar 10, 2021

BPO 43463
Nosy @gvanrossum, @larryhastings, @ericvsmith, @gvanrossum, @The-Compiler, @ilevkivskyi, @JelleZijlstra, @Fidget-Spinner, @AlexWaygood, @antonagestam

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-03-10.14:38:41.071>
labels = ['type-bug', 'library', '3.9', '3.10']
title = 'typing.get_type_hints with TYPE_CHECKING imports / getting hints for single argument'
updated_at = <Date 2022-03-20.19:21:43.531>
user = 'https://github.com/The-Compiler'

bugs.python.org fields:

activity = <Date 2022-03-20.19:21:43.531>
actor = 'AlexWaygood'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-03-10.14:38:41.071>
creator = 'The Compiler'
dependencies = []
files = []
hgrepos = []
issue_num = 43463
keywords = []
message_count = 15.0
messages = ['388436', '388444', '388556', '388578', '388749', '388750', '388751', '388756', '391210', '391225', '391229', '391251', '391254', '391257', '415627']
nosy_count = 10.0
nosy_names = ['gvanrossum', 'larry', 'eric.smith', 'Guido.van.Rossum', 'The Compiler', 'levkivskyi', 'JelleZijlstra', 'kj', 'AlexWaygood', 'antonagestam']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43463'
versions = ['Python 3.9', 'Python 3.10']

@The-Compiler
Copy link
Mannequin Author

The-Compiler mannequin commented Mar 10, 2021

Consider a file such as:

    # from __future__ import annotations
    from typing import TYPE_CHECKING, Union, get_type_hints

    if TYPE_CHECKING:
        import types

    def fun(a: 'types.SimpleNamespace', b: Union[int, str]):
        pass

    print(fun.__annotations__)
    print(get_type_hints(fun))

When running this, typing.get_type_hints fails (as you would expect):

    Traceback (most recent call last):
      File "/home/florian/tmp/x.py", line 11, in <module>
        print(get_type_hints(fun))
      File "/usr/lib/python3.9/typing.py", line 1449, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.9/typing.py", line 283, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.9/typing.py", line 539, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'types' is not defined

However, in my case I'm not actually interested in the type of 'a', I only need the type for 'b'. Before Python 3.10 (or the __future__ import), I can do so by getting it from __annotations__ directly.

With Python 3.10 (or the __future__ import), this doesn't seem to be possible anymore - I'd need to either evaluate the 'Union[int, str]' annotation manually (perhaps calling into private typing.py functions), or maybe work around the issue by passing some magical dict-like object as local/globals which ignores the NameError. Both of those seem suboptimal.

Thus, I'd like a way to either:

  1. Ignore exceptions in get_type_hints and instead get something like a typing.Unresolvable['types.SimpleNamespace'] back
  2. Have something like a typing.get_argument_type_hints(fun, 'b') instead, allowing me to get the arguments one by one rather than resolving the whole thing
  3. Have a public API to resolve a string type annotation (i.e. the equivalent of typing._eval_type)

@The-Compiler The-Compiler mannequin added 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 10, 2021
@gvanrossum
Copy link
Member

Maybe return the original string?

@The-Compiler
Copy link
Mannequin Author

The-Compiler mannequin commented Mar 12, 2021

I suppose returning the string unchanged would work as well. However, "Errors should never pass silently." :)

Perhaps that with a ignore_nameerror=True or switch or so would work?

@gvanrossum
Copy link
Member

If we add a new flag to ignore errors it's difficult to write code that
works in 3.9 as well. And given the use case I have doubts that "Errors
should never pass silently" is really the right Zen line to focus on. I'd
rather go for "Simple is better than complex" or "practicality beats
purity".

@The-Compiler
Copy link
Mannequin Author

The-Compiler mannequin commented Mar 15, 2021

Fair points.

As an aside, I'm also wondering how inspect.Parameter.annotation should interact with the changes in Python 3.10? That used to be the canonical way (as far as I'm aware) of getting a single argument's type annotation (other than getting it from __annotations__ manually), but with PEP-563 now would always return a (probably not very useful?) string.

@gvanrossum
Copy link
Member

Please open a separate issue for that.

@Fidget-Spinner
Copy link
Member

@florian,

IIUC inspect.signature auto-resolves string annotations to typing.ForwardRef internally from 3.10 onwards. It's mentioned in the what's new for PEP-563 https://docs.python.org/3.10/whatsnew/3.10.html#pep-563-postponed-evaluation-of-annotations-becomes-default

If it fails, it will just give the string. So the only place where inspect.signature might start giving you different output is if you previously defined a function like so:

def foo(a: 'MyType'): ...

And you expected inspect.signature.paramters to be [<Parameter "a: 'MyType'">] (the string). However if MyType is in globals()/locals(), you'll instead get [<Parameter "a: MyType">] in 3.10.

FWIW someone already reported that in bpo-43355.

@The-Compiler
Copy link
Mannequin Author

The-Compiler mannequin commented Mar 15, 2021

Ah, I wasn't aware of that, thanks for the pointer! So what inspect does internally is:

    def _get_type_hints(func, **kwargs):
        try:
            return typing.get_type_hints(func, **kwargs)
        except Exception:
            # First, try to use the get_type_hints to resolve
            # annotations. But for keeping the behavior intact
            # if there was a problem with that (like the namespace
            # can't resolve some annotation) continue to use
            # string annotations
            return func.__annotations__

Which means there's even some "prior art" there already falling back to a string when the annotation couldn't be resolved. Doing so in typing.get_type_hints on a per-argument basis would thus also make inspect more consistent:

Right now,

print(repr(inspect.signature(fun).parameters['b'].annotation))

in my example returns a string, but when changing the annotation for a, the returned annotation for b is now magically a typing.Union object.

(I personally would indeed expect inspect to resolve those annotations, but yeah, let's keep that in bpo-43355.)

@gvanrossum
Copy link
Member

Hey Larry, it would seem that PEP-649 as currently specified would make it impossible to access annotations via the inspect module in cases where x.__annotations__ raises (because one of the annotations references an undefined variable).

I really think that we need *some* way of accessing partial annotations. Even just leaving the failing key out of __annotations__ (but keeping other keys if their annotation works) would be better than failing to return an __annotations__ dict at all.

@larryhastings
Copy link
Contributor

Hey Larry, it would seem that PEP-649 as currently specified would make it impossible to access annotations via the inspect module in cases where x.__annotations__ raises (because one of the annotations references an undefined variable).

That's true. If PEP-649 is accepted, inspect.signature() might want to catch NameError when examining __annotations__ on the object. Though I'm not sure what it should do when the exception is raised.

I really think that we need *some* way of accessing partial annotations. Even just leaving the failing key out of __annotations__ (but keeping other keys if their annotation works) would be better than failing to return an __annotations__ dict at all.

Unfortunately I don't agree--"errors should never pass silently." Silently omitting the failed annotation seems like it would be a bad experience. What if the value you needed from the annotation was the one that was omitted? Now you have a mystery obscuring your existing problem.

There is a PR against PEP-649 specifically to suppress NameErrors:

https://github.com/larryhastings/co_annotations/pull/3

I haven't merged the PR as I don't agree with it.

@gvanrossum
Copy link
Member

If the NameError were to leave behind a marker wrapping the string, that might allow the best of both worlds -- errors don't pass *silently* (an error value is returned) and you can still inspect the other annotations.

@larryhastings
Copy link
Contributor

It would also cause the code generated for the annotations function to balloon--the calculation of every value would have to be wrapped in a try/except, and for what I assume is an obscure use case. I'm already getting some pushback on the code generated with PEP-649 as it is.

My goal in designing PEP-649 was to take stock semantics and time-shift the evaluation of the annotations, adding as little opinion as possible. I think catching NameErrors is too opinionated, much less catching every plausible error.

Still, the approach is mentioned in PEP-649. I assume the Steering Committee rules is aware this option exists. If they rule in favor of adding PEP-649, but stipulate that this feature is necessary, I will of course implement it.

@gvanrossum
Copy link
Member

I thought Jelle’s solution would avoid code bloat — a dedicated opcode. It can do whatever is decided.

@larryhastings
Copy link
Contributor

I admit I hadn't looked that closely at Jelle's PR. You're right, its effects on code size should be minimal.

If I'm reading it correctly, Jelle's PR would suppress NameError by replacing the failed value with a new "AnnotationName" object. It wouldn't handle any other exceptions (e.g. ValueError).

Also, as currently implemented it doesn't permit getting attributes or indexing into the AnnotationName--if "o" is the object we failed to look up, "o.value" and "o[1]" would raise exceptions. The AnnotationName could be extended with __getattr__ and __getitem__, but this makes it even more opinionated.

Also, the PR only adds LOAD_ANNOTATION_GLOBAL; it would presumably also need LOAD_ANNOTATION_NAME and LOAD_ANNOTATION_CLASSDEREF. (I don't think we'd need LOAD_ANNOTATION_LOCAL.)

I'll file these comments on the PR.

@antonagestam
Copy link
Mannequin

antonagestam mannequin commented Mar 20, 2022

As a consumer of get_type_hints() I think it'd be valuable to even have partially resolved types. My use case is that I provide an Annotated alias with a marker, and all I care about when inspecting user type hints is whether or not the arguments of an Annotated type contains my marker object. So ideally the fallback to an unresolved string or a sentinel object such as the proposed typing.Unresolvable should happen at the "lowest resolvable level" so that what can be resolved isn't lost.

By example, I'm saying that I think that this code:

    marker = object()

    def dec(cls):
        print(get_type_hints(cls))
        return cls

    @dec
    class A(abc.ABC):
        forward: Annotated[B, marker]

    class B:
        ...

Should produce:

{"forward": Annotated[Unresolvable["B"], marker]}

I guess this would apply in situations where for instance a part of a union isn't resolvable too. If we have a union A|B where A is resolvable and B isn't, it should be resolved to:

A | Unresolvable["B"]

And not to:

Unresolvable["A | B"]

(I think for this perspective it's irrelevant whether unresolved types have a sentinel type or are just represented as strings).

(Here's the library that's my use case for the curious: https://github.com/antonagestam/abcattrs)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants