Skip to content

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchakaserhiy-storchaka commented Aug 20, 2021

Because setting non-empty _name affects other behavior.

In most cases __name__ can be derived from __origin__.__name__.

https://bugs.python.org/issue44524

Becase setting non-empty _name affects other behavior. In most cases __name__ can be derived from __origin__.__name__.
@gvanrossum
Copy link
Member

What does Ken Jin say?

Copy link
Member

@Fidget-SpinnerFidget-Spinner left a comment

Choose a reason for hiding this comment

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

LGTM. Personally I don't think setting _name on _SpecialForms will break anything (because they aren't subclassable, and any methods they need are overridden, so their MRO doesn't matter). But I've been wrong on multiple occasions at this point 😉 , so let's just be safe. From a technical standpoint, Serhiy's solution is much cleaner (and faster) too.

PS @serhiy-storchaka , you can do the same for Annotated and copy the test over from my PR #27841. I'm closing that so we don't have to deal with merging multiple PRs.

def__getattr__(self, attr):
ifattrin{'__name__', '__qualname__'}:
returnself._name
returnself._nameorself.__origin__.__name__

Choose a reason for hiding this comment

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

This solution is so nice. I wish I'd thought about this a little deeper when I proposed the original one:(.

Anyways if I'm not wrong, __origin__ will always be set on all GenericAlias to some type, so this should always succeed.

Comment on lines +524 to +525
return_UnionGenericAlias(self, parameters, name="Optional")
return_UnionGenericAlias(self, parameters)
Copy link
Member

@Fidget-SpinnerFidget-SpinnerAug 21, 2021

Choose a reason for hiding this comment

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

Will there be a subtle difference between how the two behaves since one sets name and the other doesn't?

I would think not but I'm not sure.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Setting _name affects subclassing and pickling/copying. Union[] and Optional[] are not subclassable, and special implementation of __reduce__ was added in previous commits. If there are other difference, I do not know.

@serhiy-storchakaserhiy-storchaka merged commit 4ceec49 into python:mainAug 21, 2021
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@serhiy-storchakaserhiy-storchaka deleted the typing-specialform-name branch August 21, 2021 06:48
@bedevere-bot
Copy link

GH-27871 is a backport of this pull request to the 3.10 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Aug 21, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 21, 2021
) Because setting non-empty _name affects behavior of other code. In most cases __name__ can be derived from __origin__.__name__. (cherry picked from commit 4ceec49) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@serhiy-storchaka
Copy link
MemberAuthor

you can do the same for Annotated and copy the test over from my PR #27841.

It is easier to merge them separately. There are no conflicts.

serhiy-storchaka added a commit that referenced this pull request Aug 21, 2021
…H-27871) Because setting non-empty _name affects behavior of other code. In most cases __name__ can be derived from __origin__.__name__. (cherry picked from commit 4ceec49) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@farcatfarcatmannequin mentioned this pull request Apr 14, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@serhiy-storchaka@gvanrossum@miss-islington@bedevere-bot@Fidget-Spinner@the-knights-who-say-ni