8000 GH-96079 Fix missing field name for _AnnotatedAlias by iyume · Pull Request #96080 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content
8000

GH-96079 Fix missing field name for _AnnotatedAlias #96080

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

Merged
merged 5 commits into from
Aug 31, 2022

Conversation

iyume
Copy link
Contributor
@iyume iyume commented Aug 18, 2022

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@JelleZijlstra
Copy link
Member

Can you add a test case showing what user-visible behavior you're changing here?

@iyume

This comment was marked as outdated.

@hauntsaninja hauntsaninja added the pending The issue will be closed if no feedback is provided label Aug 19, 2022
Copy link
Member
@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I've found a user-visible effect of the change.

Without this change, the __module__ attribute of an Annotated type is "builtins", which is incorrect. With this fix, it's "typing", correctly.

Main:

>>> import typing
>>> typing.Annotated[int, "foo"].__module__
'builtins'
>>>

Patched:

>>> typing.Annotated[int, "foo"].__module__ 
'typing'
>>>

Hopefully that can inspire a test?

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@iyume iyume requested review from gvanrossum and removed request for JelleZijlstra and Fidget-Spinner August 31, 2022 13:10
Comment on lines +2135 to +2136
def __mro_entries__(self, bases):
return (self.__origin__,)
Copy link
Member

Choose a reason for hiding this comment

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

What happens without this? It seems kind of random to me (but I didn't try to run the tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gvanrossum First about __module__: it is not truly covered by test:

typing.Annotated[Any, "Annotation"]: 'Annotated',

Here typing.Any.__module__ is typing, so passed, but errors in int. It is directly caused by:

cpython/Lib/typing.py

Lines 1357 to 1358 in c7f2686

if not name:
self.__module__ = origin.__module__

The idea of this PR is to add _name attribute to _AnnotatedAlias instance, since _AnnotatedAlias.__origin__ is just the first argument and nowhere to determind where Annotated annotation from.

But with _name attribute, _GenericAlias will think the __origin__ is generic ABCs, Generic will finally add to mros.

cpython/Lib/typing.py

Lines 1270 to 1280 in c7f2686

def __mro_entries__(self, bases):
res = []
if self.__origin__ not in bases:
res.append(self.__origin__)
i = bases.index(self)
for b in bases[i+1:]:
if isinstance(b, _BaseGenericAlias) or issubclass(b, Generic):
break
else:
res.append(Generic)
return tuple(res)

So __mro_entries__ overridden is required.

Copy link
Member
@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Okay, my spider-sense is still tingling, but it does seem all right. I'll land, but I'm hesitant to backport to 3.11.

@gvanrossum gvanrossum merged commit 0cd33e1 into python:main Aug 31, 2022
@AA-Turner AA-Turner removed the pending The issue will be closed if no feedback is provided label Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0