-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
gh-101688: Implement types.get_original_bases #101827
Merged
Merged
Changes from 40 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
50707ba
Implement typing.get_orig_class and get_orig_bases
Gobot1234 4fef9f8
📜🤖 Added by blurb_it.
blurb-it[bot] 4cb193d
Apparently my lsp wasn't working
Gobot1234 44d3ae9
Removing trailing whitespace
Gobot1234 59e9ac2
Fix test failure
Gobot1234 20024e9
Merge branch 'main' into orig_class-and-bases
Gobot1234 769fdbd
Fix typo
Gobot1234 7194d3e
Respond to initial review
Gobot1234 eeb4475
Remove trailing ws
Gobot1234 198dc08
Respond to second round of review
Gobot1234 695cf93
Fix typo
Gobot1234 cc11033
Fix last few comments
Gobot1234 a66282b
Merge remote-tracking branch 'upstream/main' into orig_class-and-bases
Gobot1234 d3768ba
Merge branch 'main' into orig_class-and-bases
Gobot1234 581a91a
Apply suggestions from code review
Gobot1234 fb15427
Remove typing.get_orig_class
Gobot1234 96b9536
Merge remote-tracking branch 'origin/orig_class-and-bases' into orig_…
Gobot1234 6c38e4d
Remove old test
Gobot1234 372bd6b
Rename to get_original_bases
Gobot1234 a6fe8ac
Autoformatting is fun
Gobot1234 2b71b74
Fix more suggestions
Gobot1234 e7635c6
Remove rawstring
Gobot1234 42e1668
Apply suggestions from code review
Gobot1234 1040478
Fix `test_types` failures
AlexWaygood 8085258
Apply suggestions from code review
Gobot1234 b9bf4fd
Add a whatsnew entry
Gobot1234 ee59cbd
Update Lib/types.py
Gobot1234 e5a91a5
Update Doc/library/types.rst
Gobot1234 7bad429
Merge branch 'main' into orig_class-and-bases
AlexWaygood 2a39055
Merge branch 'main' into orig_class-and-bases
Gobot1234 1ea82be
Remove trailing WS
Gobot1234 1cb14c2
Fix more trailing WS
Gobot1234 6f06c52
Fallback to __bases__
Gobot1234 689267a
Update missing docs
Gobot1234 1ae16ea
Apply suggestions from code review
Gobot1234 3a8619a
Update Doc/library/types.rst
Gobot1234 e1d55d6
Merge remote-tracking branch 'upstream/main' into orig_class-and-bases
AlexWaygood bab8cb3
Small tweaks to docs
AlexWaygood fb9ef70
Use `assert` in docs examples
AlexWaygood dc433c4
Document type.__orig_bases__
Gobot1234 4268b74
Revert "Document type.__orig_bases__"
Gobot1234 9f54ac1
Update Doc/library/types.rst
Gobot1234 c122b23
Hone docs more
AlexWaygood 2134053
Merge remote-tracking branch 'upstream/main' into orig_class-and-bases
AlexWaygood cb21a65
More tests and docs following #103698
AlexWaygood File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
Misc/NEWS.d/next/Library/2023-02-11-15-01-32.gh-issue-101688.kwXmfM.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Implement :func:`types.get_original_bases` to provide further introspection | ||
for types. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "modification by
__mro_entries__
" refer to? In my mind that is a read-only function that doesn't modify anything.But I'm probably missing another subtle distinction. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't modify anything itself I guess, but it does cause the
__bases__
to be changed when creating the type.Does this reflect the nature of the internals here better do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't follow. Can you point me to the code where
__mro_entries__
modifies something?FWIW In general it's better not to describe things in terms of "internals", but instead refer to the specification of an object or operation in the documentation. (And once something is documented it's no longer internal, even if it has a dunder name.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the following class:
Naively, from looking at the code, you might expect the
__bases__
attribute ofFoo
to be equal to(Generic[T],)
, if you're not familiar with how typing generics and__mro_entries__
work. But it's not:This is because Python looks at
Generic[T]
(an instance oftyping._GenericAlias
), sees it's not an instance oftype
, looks for an__mro_entries__
method onGeneric[T]
, finds that it does have an__mro_entries__
method, and callsGeneric[T].__mro_entries__((Generic[T],))
to determine what theFoo
class's "real" bases should be:>>> Generic[T].__mro_entries__((Generic[T],)) (<class 'typing.Generic'>,)
So whereas the
Foo
class's "original bases" are(Generic[T],)
, the class's "actual bases, after__mro_entries__
has been called on all bases that aren't instances oftype
and have__mro_entries__
methods on them" are(Generic,)
.That's the information we're trying to get across here. The trouble is getting it across in a way that's both concise and clear!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I have to admit I was again fooled by misreading. :-( I might as well come clear, I didn't remember the ugly PEP 560 mechanism and thought you were referencing
__mro__
. :-(Maybe we can work a PEP 560 reference in the text, e.g. "Return the tuple of base classes originally passed to class creation, before they were replaced the PEP 560 mechanism using
__mro_entries__
."I'm really sorry for being so dense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you did ask me for a review. :-) PEP 560 is one of the most obscure mechanisms in all of typing, so deserves a mention whenever it is used. Also, perhaps a mention that this essentially just returns
__orig_bases__
, if it exists, might be helpful.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let's add the PEP-560 link then. Though I don't know how helpful mentioning
__orig_bases__
would be for most users, since the attribute is currently completely undocumented AFAIK (other than an offhand reference in PEP-560, shortly above a paragraph that says that the attribute should remain an implementation detail of CPython that shouldn't be used outside the stdlib)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I very much value your feedback — this is a tough function to describe!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the use cases for the new addition are all people who are trying to use
__orig_bases__
! They should be able to find this through searching.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, fair point :-)