Skip to content

[3.14] annotationlib - get_annotations returns an empty annotations dict if an AttributeError is raised when __annotations__ is accessed #125618

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

Closed
DavidCEllis opened this issue Oct 16, 2024 · 17 comments · Fixed by #132818
Assignees
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@DavidCEllis
Copy link
Contributor

DavidCEllis commented Oct 16, 2024

Bug report

Bug description:

If there's an annotation with an incorrect attribute access, the AttributeError causes get_annotations to return an empty dictionary for the annotations instead of failing or returning a ForwardRef.

import typing
from annotationlib import get_annotations, Format

class Example2:
    real_attribute: typing.Any
    fake_attribute: typing.DoesNotExist

new_ann = get_annotations(Example2, format=Format.FORWARDREF)

print(f"{new_ann=}")

# This should fail, but instead returns an empty dict
value_ann = get_annotations(Example2, format=Format.VALUE)

print(f"{value_ann=}")

string_ann = get_annotations(Example2, format=Format.STRING)

print(f"{string_ann=}")

Output

new_ann={}
value_ann={}
string_ann={'real_attribute': 'typing.Any', 'fake_attribute': 'typing.DoesNotExist'}

I think this is due to _get_dunder_annotations catching AttributeError and returning an empty dict, intended for static types but catching this by mistake.

CPython versions tested on:

3.14

Operating systems tested on:

Linux

Linked PRs

@DavidCEllis DavidCEllis added the type-bug An unexpected behavior, bug, or error label Oct 16, 2024
@JelleZijlstra JelleZijlstra self-assigned this Oct 16, 2024
@JelleZijlstra
Copy link
Member

Thanks, that's an interesting case. Not immediately clear to me how we can fix this; it's not clear how to tell where the AttributeError came from. Maybe we should make static types return an empty dict in their __annotations__ descriptor instead of throwing AttributeError.

@DavidCEllis
Copy link
Contributor Author

I was actually testing it to see if I'd get a _Stringifier again or a ForwardRef based on the other issue and was a bit surprised when I got nothing instead.

@JelleZijlstra
Copy link
Member

In the FORWARDREF format we try to return .__annotations__ first. If that succeeds, we're done. And in this case, it arguably does succeed.

@DavidCEllis
Copy link
Contributor Author

If you directly access Example2.__annotations__ you get the AttributeError though. The 'success' is entirely because this error is being silenced.

I'd expect VALUE to raise this exception. I'd expect FORWARDREF to either also error in this way or return a ForwardRef for the (currently) invalid attribute.

Traceback (most recent call last):
  File "/home/david/src/scratch/annotation_issue.py", line 34, in <module>
    Example2.__annotations__
  File "/home/david/src/scratch/annotation_issue.py", line 19, in __annotate__
    fake_attribute: typing.DoesNotExist
                    ^^^^^^^^^^^^^^^^^^^
  File "/home/david/src/cpython/Lib/typing.py", line 3829, in __getattr__
    raise AttributeError(f"module {__name__!r} has no attribute {attr!r}")
AttributeError: module 'typing' has no attribute 'DoesNotExist'

@DavidCEllis
Copy link
Contributor Author

So I came back to this and realised that this is actually worse if some of the types are from a submodule that's only imported in a TYPE_CHECKING block.

For instance:

ex/__init__.py

class SomeClass:
    ...

ex/protocols.py

class SomeProtocol:
    ...

demo.py

import typing
from dataclasses import dataclass

import ex

if typing.TYPE_CHECKING:
    import ex.protocols

@dataclass
class Demo:
    a: ex.SomeClass | None = None
    b: ex.protocols.SomeProtocol | None = None

print(Demo())

python -c "import demo"

Demo()

python -c "import ex.protocols; import demo"

Demo(a=None, b=None)

So the definition of the dataclass can change depending on whether ex.protocols is imported elsewhere before this class is defined.

@picnixz picnixz added stdlib Python modules in the Lib dir 3.14 new features, bugs and security fixes topic-typing labels Mar 7, 2025
@DavidCEllis
Copy link
Contributor Author

DavidCEllis commented Apr 8, 2025

Was writing a test for this and found another variation of the bug along with a confusing error message which I'd consider a separate bug.

in 3.13 if you try inspect.get_annotations on an instance you get an error message:

>>> inspect.get_annotations(X())
TypeError: <__main__.X object at 0x70591bb8c980> is not a module, class, or callable.

However in the current 3.14 main there are no such guards which leads to some unexpected behaviour.

The first case is the same bug as we have here in a different presentation.

from annotationlib import get_annotations, Format

class Example:
   a: int

ex = Example()

print(f"{get_annotations(ex) = }")
print(f"{get_annotations(Example) = }")
print(f"{get_annotations(ex) = }")

Output:

get_annotations(ex) = {}
get_annotations(Example) = {'a': <class 'int'>}
get_annotations(ex) = {'a': <class 'int'>}

The bug is this function:
https://github.com/python/cpython/blob/53908bd7905b849e110d2c6f4bce739bff037146/Lib/annotationlib.py#L833C1-L847C21

It assumes AttributeError means the object is a "static type" and returns an empty dictionary1. As the instance ex doesn't have an __annotations__ attribute, it raises AttributeError and you see this empty dict. However, after get_annotations has been called on Example, __annotations__ has been created on the class and is now retrieved.

>>> class Example:
...     a: int
...     
>>> ex = Example()
>>> ex.__annotations__
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    ex.__annotations__
AttributeError: 'Example' object has no attribute '__annotations__'. Did you mean: '__annotate__'?
>>> Example.__annotations__
{'a': <class 'int'>}
>>> ex.__annotations__
{'a': <class 'int'>}

Edit: Accidentally linked to the wrong function - it should be _get_dunder_annotations.

Footnotes

  1. Not actually the case, I realised later that this occurs in the else branch. Same outcome though.

@DavidCEllis
Copy link
Contributor Author

I wrote some test cases for this error.

My initial thought is that _get_dunder_annotations should return None instead of an empty dict to indicate that __annotations__ has not been found, and then usages of this function should check if __annotate__ exists before potentially trying to call it. Probably wrapping all of this in its own function?

The test cases - STRING passes but is included for completion.

class TestGetAnnotationsAttributeError(unittest.TestCase):
    def test_value(self):
        class HasError:
            missing_attribute: base.missing_attribute

        # Before creating 'base'
        with self.assertRaises(NameError):
            annotationlib.get_annotations(HasError)

        base = object()

        # Should now be an attributeerror for base.missing_attribute
        with self.assertRaises(AttributeError):
            annotationlib.get_annotations(HasError)

    def test_forwardref(self):
        class HasError:
            missing_attribute: base.missing_attribute

        missing_ref = support.EqualToForwardRef(
            "base.missing_attribute",
            owner=HasError,
            is_class=True,
        )

        # before creating 'base'
        forward_annos = annotationlib.get_annotations(HasError, format=Format.FORWARDREF)

        self.assertEqual(
            forward_annos,
            {"missing_attribute": missing_ref}
        )

        # Create base but the annotations should be the same
        base = object()

        forward_annos = annotationlib.get_annotations(HasError, format=Format.FORWARDREF)

        self.assertEqual(
            forward_annos,
            {"missing_attribute": missing_ref}
        )


    def test_string(self):
        class HasError:
            missing_attribute: base.missing_attribute

        string_annos = annotationlib.get_annotations(HasError, format=Format.STRING)

        self.assertEqual(
            string_annos,
            {"missing_attribute": "base.missing_attribute"}
        )

        base = object()

        string_annos = annotationlib.get_annotations(HasError, format=Format.STRING)

        self.assertEqual(
            string_annos,
            {"missing_attribute": "base.missing_attribute"}
        )

@JelleZijlstra
Copy link
Member

#132195 makes some related changes.

@DavidCEllis
Copy link
Contributor Author

DavidCEllis commented Apr 8, 2025

From these tests, #132195 makes test_value behave as expected - NameError in the first case and AttributeError in the second.

The test_forwardref as written above still fails as it seems Format.FORWARDREF now raises an AttributeError but I think it should return a ForwardRef the same as it would be before the import. The error is better than the incorrect result, but I don't think this format should fail in this way.

[Edit: rephrasing for clarity]

@JelleZijlstra
Copy link
Member

The behavior with #132195 seems much more reasonable to me.

I don't immediately see a way to get the behavior you want, where if we see x.y and x exists but does not have an attribute y, we get a ForwardRef("x.y"). When we evaluate x, we must return the actual x object, so that the annotations contain the "real" object whenever possible. But that means that when we get to evaluating x.y, we can't hook into attribute access any more.

@DavidCEllis
Copy link
Contributor Author

Yes I'm not sure how to get there as after you've managed to pull the attribute out of the 'fake globals' dict correctly. At least without needing to make the fake globals dict do something very ugly.

It does bug me that this is an area where get_annotations(..., format=Format.FORWARDREF) fails though, even if it is possible to work around the failures1. My impression on originally learning about the new deferred annotations was that it was intended to be as safe as accessing __annotations__ currently is in 3.13, but it seems only Format.STRING is actually guaranteed to 'work'.

Footnotes

  1. For example in the ex.protocols demo, doing import ex.protocols as ex_protocols inside the TYPE_CHECKING block instead.

@DavidCEllis
Copy link
Contributor Author

There might be an internal reason not to do this1 but one possible solution would be to provide an empty dict as you do for STRING. Make everything a _Stringifier and then at the end when you switch the class from _Stringifier to ForwardRef, evaluate all of the references you can and handle whether to return the original object or a ForwardRef at this point?

This is almost certainly slower but could potentially be used as a fallback if the current method failed.

Side note: A format that always returned ForwardRef for everything might be useful. Was that discussed at any point? The use case would be constructing things dataclass-like without actually evaluating the values during construction. It would also mean not needing to check for ForwardRef as everything would be one.

Footnotes

  1. I thought there might be some cases where the current method evaluated something that this change didn't but a quick prototype of this method didn't break any existing tests.

@JelleZijlstra
Copy link
Member

I believe that would break cases like x: list[nonexistent]. Currently that produces list[ForwardRef('nonexistent')]; your suggestion would produce ForwardRef('list[nonexistent]'). If we don't have a test for that case, we should!

We could still use this approach as a fallback in cases where evaluation otherwise raises AttributeError; that seems safe.

A format that always returned ForwardRef for everything might be useful. Was that discussed at any point?

I don't think this has been considered. I'm not sure I fully understand the use case. If you don't need anything evaluated, why are you evaluating it at all? If you need something dataclass-like that doesn't need to look at annotations at all, you can just forward from the wrapper's __annotate__ to the original.

@DavidCEllis
Copy link
Contributor Author

I believe that would break cases like x: list[nonexistent]. Currently that produces list[ForwardRef('nonexistent')]; your suggestion would produce ForwardRef('list[nonexistent]'). If we don't have a test for that case, we should!

Ah, you're correct it does produce this currently and also that there doesn't seem to be a test for it.

Would have been nice if there was an .evaluate_the_stuff_you_can() method that could evaluate a forwardref against a 'fake globals' dict and partially resolve the reference but a quick attempt to do so this morning failed.

A format that always returned ForwardRef for everything might be useful. Was that discussed at any point?

I don't think this has been considered. I'm not sure I fully understand the use case. If you don't need anything evaluated, why are you evaluating it at all? If you need something dataclass-like that doesn't need to look at annotations at all, you can just forward from the wrapper's __annotate__ to the original.

For this dataclass-like I mostly1 don't care about the annotation value for class construction but I do need the keys and - as far as I know - you can't get one without the other.

Ideally though, I'd also like to be able to keep a type attribute on my Field class that does evaluate the type that would then look very roughly like this:

class FieldLike:
    ...
    _type: ForwardRef
    @property
    def type(self):
        try:
            return self._type.evaluate()
        except NameError, AttributeError, KeyError:
            return self._type

Then assume you have a class like this:

@my_dataclass
class Example:
    attrib: defer_imports.bigmodule.SomeType

With an ALWAYS_FORWARDREF mode you could both construct the class without evaluating the reference and triggering the import and also still have a .type property on Field that correctly gave the type on-demand. Much like dataclasses itself the fields list will also contain all inherited fields and their types so it's a bit more convoluted to try to find the appropriate __annotate__ method after the fact.

Footnotes

  1. The exceptions are currently ClassVar and KW_ONLY, but assume in this case that there's an option to ignore all values.

@JelleZijlstra
Copy link
Member

The behavior of your original example is now that FORWARDREF and VALUE fail with a sensible error AttributeError: module 'typing' has no attribute 'DoesNotExist' and STRING also behaves correctly.

Are you interested in contributing a PR for your suggestion to make the FORWARDREF format fall back to the approach in #125618 (comment) on exceptions? I think that's a good idea.

@DavidCEllis
Copy link
Contributor Author

Sure, I can probably find some time next week to have a look at that. Possibly another look to see if there's a way to do that and still get list[ForwardRef('nonexistent')] instead of ForwardRef('list[nonexistent]')?

@JelleZijlstra
Copy link
Member

Thanks!

a way to do that and still get list[ForwardRef('nonexistent')] instead of ForwardRef('list[nonexistent]')?

I think a way to do that could be to add a format argument to ForwardRef.evaluate. STRING is not very useful but can be implemented trivially. FORWARDREF would still run eval(), but using the special globals environment like we use in call_annotate_function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants