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

bpo-42317: Improve docs of typing.get_args concerning Union #23254

Merged
merged 4 commits into from
Nov 16, 2020

Conversation

Dominik1123
Copy link
Contributor

@Dominik1123 Dominik1123 commented Nov 12, 2020

https://bugs.python.org/issue42317

Automerge-Triggered-By: GH:gvanrossum

@@ -1706,6 +1706,8 @@ Introspection helpers
For a typing object of the form ``X[Y, Z, ...]`` these functions return
``X`` and ``(Y, Z, ...)``. If ``X`` is a generic alias for a builtin or
:mod:`collections` class, it gets normalized to the original class.
If ``X`` is a :class:`Union`, the order of ``(Y, Z, ...)`` may be different
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If ``X`` is a :class:`Union`, the order of ``(Y, Z, ...)`` may be different
If ``X`` is a :class:`Union` contained in another generic type, the order of ``(Y, Z, ...)`` may be different

@Fidget-Spinner
Copy link
Member

I don't really like the current proposed change because it's too broad, it may cause users to worry that all nested types are affected, when in reality it's only Union.

Instead of:

If X is a generic type, the returned objects (Y, Z, ...) might not be identical to the ones used in the form X[Y, Z, ...] due to type caching.

Guido's change seems a lot more apt:

If X is a :class:Union contained in another generic type, the order of (Y, Z, ...) may be different

A quick search through the typing code tells me that only the __hash__ and __eq__ methods of Union are order independent, no other typing type will suffer from this behavior when nested in another Generic.

@Dominik1123
Copy link
Contributor Author

I don't really like the current proposed change because it's too broad, it may cause users to worry that all nested types are affected, when in reality it's only Union.

It's as broad as the promise of type caching goes, so I don't think it's too broad. But I agree that it's broader than necessary since indeed, at the moment, it requires a Union to be involved. Initially I thought it would also affect Literal, but apparently it doesn't:

>>> Literal[1, 2] == Literal[2, 1]
False

(Though I don't see why the two would be unequal; but that's a different concern)

Instead of:

If X is a generic type, the returned objects (Y, Z, ...) might not be identical to the ones used in the form X[Y, Z, ...] due to type caching.

Guido's change seems a lot more apt:

If X is a :class:Union contained in another generic type, the order of (Y, Z, ...) may be different

So I just revert back to Guido's version?

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Nov 13, 2020

(Though I don't see why the two would be unequal; but that's a different concern)

Wow good catch, that looks like it may be a bug.

So I just revert back to Guido's version?

Maybe a combination of both yours and Guido's wording:

If `X` is a :class:`Union` contained in another generic type, the order of `(Y, Z, ...)` may be different due to type caching.

Btw, thank you for the PR!

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.

Thanks, LG!

@miss-islington
Copy link
Contributor

Thanks @Dominik1123 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 16, 2020
…-23254)

(cherry picked from commit c3b9592)

Co-authored-by: Dominik1123 <15989985+Dominik1123@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Nov 16, 2020
@bedevere-bot
Copy link

GH-23307 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request Nov 16, 2020
(cherry picked from commit c3b9592)

Co-authored-by: Dominik1123 <15989985+Dominik1123@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants