gh-101688: Implement types.get_original_bases #101827

Gobot1234 · 2023-02-11T14:59:58Z

Implements the methods described.

A couple of questions:

Should there be checking of the return types of the functions since they could be anything?
Should we add __orig_class__ to the slots of all the types that currently don't have them to provide better introspection support (currently get_orig_class(list[int]()) is None)

Issue: Add typing.get_orig_bases and typing.get_orig_class #101688

sobolevn

👍

Doc/library/typing.rst

Lib/test/test_typing.py

Lib/typing.py

Gobot1234 · 2023-02-18T00:20:44Z

Error looks unrelated?

sobolevn · 2023-02-18T08:10:49Z

Yes, it is.

Doc/library/typing.rst

# Conflicts: # Lib/test/test_typing.py

Lib/test/test_types.py

Lib/types.py

Misc/NEWS.d/next/Library/2023-02-11-15-01-32.gh-issue-101688.kwXmfM.rst

Lib/types.py

AlexWaygood

Final round of nits:

Doc/library/types.rst

Lib/test/test_types.py

Lib/types.py

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood

LGTM, thank you! I'll wait a few days before merging, in case @JelleZijlstra or @gvanrossum have any objections or nits :)

gvanrossum · 2023-04-11T23:04:32Z

I am very confused.

The intent of the __orig_bases__ attribute was that that itself should be the "stable API". Just because it's a dunder doesn't mean it isn't stable or can't be a documented public API (in fact, many dunders are both, from __add__ to __init__).

Moreover, the examples in the docs assign values to __bases__ attributes, but then magically retrieve the previous bases -- I don't understand how that would work, since there's no memory in the class of what __bases__ was before being assigned to. This is furthermore confounded because none of the tests use assignment to __bases__.

Unless I am missing something dramatically, it feels to me that this is solving a non-problem in a strange way.

Gobot1234 · 2023-04-11T23:16:17Z

The intent of the __orig_bases__ attribute was that that itself should be the "stable API". Just because it's a dunder doesn't mean it isn't stable or can't be a documented public API (in fact, many dunders are both, from __add__ to __init__).

Whilst this is true, __args__ is documented on GenericAlias objects and yet typing still has get_args(), due to the fact that GenericAlias isn't really a thing at type time and so accessing it directly on a list[str] for example doesn't work (https://mypy-play.net/?mypy=latest&python=3.11&gist=bab962613df06bb5a79fe35adc3e3d19). There is a similar sort of problem with accessing __orig_bases__, not only is it undocumented but it is hard to statically determine if any given type has the attribute so having this escape hatch serves a very useful purpose for introspection.

Moreover, the examples in the docs assign values to __bases__ attributes, but then magically retrieve the previous bases -- I don't understand how that would work, since there's no memory in the class of what __bases__ was before being assigned to. This is furthermore confounded because none of the tests use assignment to __bases__.

Double check the number of equals 😉 (if you think it might be clearer to use an assert I wouldn't be opposed)

Doc/library/types.rst

gvanrossum · 2023-04-11T23:22:13Z

Doc/library/types.rst

+.. function:: get_original_bases(cls, /)
+
+    Return the objects in the bases list of the class's definition as they
+    existed prior to any modification by :meth:`~object.__mro_entries__`. This


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. :-)

It doesn't modify anything itself I guess, but it does cause the __bases__ to be changed when creating the type.

Suggested change

existed prior to any modification by :meth:`~object.__mro_entries__`. This

existed before :meth:`~object.__mro_entries__` caused them to change. This

Does this reflect the nature of the internals here better do you think?

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.)

Consider the following class:

from typing import Generic, TypeVar T = TypeVar("T") class Foo(Generic[T]): ...

Naively, from looking at the code, you might expect the __bases__ attribute of Foo to be equal to (Generic[T],), if you're not familiar with how typing generics and __mro_entries__ work. But it's not:

>>> from typing import TypeVar, Generic >>> T = TypeVar("T") >>> class Foo(Generic[T]): ... ... >>> Foo.__bases__ (<class 'typing.Generic'>,) >>> Foo.__bases__[0] == Generic[T] False

This is because Python looks at Generic[T] (an instance of typing._GenericAlias), sees it's not an instance of type, looks for an __mro_entries__ method on Generic[T], finds that it does have an __mro_entries__ method, and calls Generic[T].__mro_entries__((Generic[T],)) to determine what the Foo 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 of type 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!

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.

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.

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)?

Well, you did ask me for a review. :-)

And I very much value your feedback — this is a tough function to describe!

Though I don't know how helpful mentioning __orig_bases__ would be for most users, since the attribute is currently completely undocumented AFAIK

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.

Hah, fair point :-)

bedevere-bot · 2023-04-11T23:22:46Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

AlexWaygood · 2023-04-12T15:41:47Z

The intent of the __orig_bases__ attribute was that that itself should be the "stable API". Just because it's a dunder doesn't mean it isn't stable or can't be a documented public API (in fact, many dunders are both, from __add__ to __init__) [...] Unless I am missing something dramatically, it feels to me that this is solving a non-problem in a strange way.

FWIW, I fully agree that that "just because it's a dunder doesn't mean it isn't stable or can't be a documented public API". For me, the biggest problem with the status quo is that most people who want to access the __orig_bases__ attribute at runtime are users of type hints. However, the __orig_bases__ attribute is only present on classes that inherit from instances of types.GenericAlias or typing._GenericAlias. Both of these classes are heavily special-cased by type checkers, and indeed typing._GenericAlias doesn't appear at all in typeshed's stubs. This means that it's impossible to add a stub for this attribute to typeshed, and so type checkers always emit errors if users try to access this attribute at runtime: python/typeshed#7827. This has been raised a few times over at typeshed, so I think it's reasonable to say that this is a pain point for users of type hints at the moment.

You could argue, "Couldn't type checkers just special-case this attribute?", and others have: microsoft/pyright#3442. However, they're (reasonably) very reluctant to do so.

As such, the proposal is to add a simple "getter" function to CPython, which we can easily add a stub for in typeshed, as a complement to the typing.get_args and typing.get_origin getter functions that already exist. This seems to me to be a reasonable solution to this problem.

gvanrossum · 2023-04-12T15:56:23Z

As such, the proposal is to add a simple "getter" function to CPython, which we can easily add a stub for in typeshed, as a complement to the typing.get_args and typing.get_origin getter functions that already exist. This seems to me to be a reasonable solution to this problem.

Yeah, I agree with this for sure.

AlexWaygood · 2023-04-12T19:47:27Z

Doc/library/types.rst

+    Return the objects in the bases list of the class's definition as they
+    existed prior to any modification by :meth:`~object.__mro_entries__`. This
+    is useful for introspecting :ref:`Generics <user-defined-generics>`.


How about this, to address @gvanrossum's feedback?

Suggested change

Return the objects in the bases list of the class's definition as they

existed prior to any modification by :meth:`~object.__mro_entries__`. This

is useful for introspecting :ref:`Generics <user-defined-generics>`.

Return the tuple of objects originally given as the bases of *cls* before

the :meth:`~object.__mro_entries__` method has been called on any bases

(following the mechanisms laid out in :pep:`560`). This is useful for

introspecting :ref:`Generics <user-defined-generics>`.

For classes that have an ``__orig_bases__`` attribute, this

function simply returns the value of ``cls.__orig_bases__``.

Isn't

For classes that have an :attr:`~type.__orig_bases__` attribute, this

going to not resolve properly?

Isn't

For classes that have an :attr:`~type.__orig_bases__` attribute, this

going to not resolve properly?

I changed my suggested change to not include that (you might need to refresh your browser if you're still seeing the old suggestion) :)

Doc/reference/datamodel.rst

This reverts commit dc433c4.

Implement typing.get_orig_class and get_orig_bases

50707ba

Gobot1234 requested review from gvanrossum, Fidget-Spinner, JelleZijlstra and AlexWaygood as code owners February 11, 2023 14:59

bedevere-bot added the awaiting review label Feb 11, 2023

bedevere-bot mentioned this pull request Feb 11, 2023

Add typing.get_orig_bases and typing.get_orig_class #101688

Open

blurb-it bot and others added 3 commits February 11, 2023 15:01

📜🤖 Added by blurb_it.

4fef9f8

Apparently my lsp wasn't working

4cb193d

Removing trailing whitespace

44d3ae9

arhadthedev added stdlib Python modules in the Lib dir topic-typing labels Feb 11, 2023

Gobot1234 added 3 commits February 11, 2023 20:13

Fix test failure

59e9ac2

Merge branch 'main' into orig_class-and-bases

20024e9

Fix typo

769fdbd

sobolevn reviewed Feb 12, 2023

View reviewed changes

Gobot1234 added 2 commits February 12, 2023 11:38

Respond to initial review

7194d3e

Remove trailing ws

eeb4475

JelleZijlstra reviewed Feb 22, 2023

View reviewed changes

Gobot1234 changed the title ~~gh-101688: Implement typing.get_orig_class and get_orig_bases~~ gh-101688: Implement typing.get_orig_class and types.get_orig_bases Feb 25, 2023

Gobot1234 added 5 commits February 25, 2023 16:43

Respond to second round of review

198dc08

Fix typo

695cf93

Fix last few comments

cc11033

Merge remote-tracking branch 'upstream/main' into orig_class-and-bases

a66282b

# Conflicts: # Lib/test/test_typing.py

Merge branch 'main' into orig_class-and-bases

d3768ba

AlexWaygood requested changes Apr 5, 2023

View reviewed changes

Lib/test/test_types.py Outdated Show resolved Hide resolved

Lib/test/test_types.py Outdated Show resolved Hide resolved

Lib/types.py Outdated Show resolved Hide resolved

Misc/NEWS.d/next/Library/2023-02-11-15-01-32.gh-issue-101688.kwXmfM.rst Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting review labels Apr 5, 2023

Gobot1234 added 3 commits April 9, 2023 11:00

Merge branch 'main' into orig_class-and-bases

2a39055

Remove trailing WS

1ea82be

Fix more trailing WS

1cb14c2

Gobot1234 requested a review from AlexWaygood April 9, 2023 11:11

AlexWaygood reviewed Apr 9, 2023

View reviewed changes

Lib/types.py Outdated Show resolved Hide resolved

gst reviewed Apr 9, 2023

View reviewed changes

Lib/types.py Outdated Show resolved Hide resolved

Gobot1234 added 2 commits April 9, 2023 19:37

Fallback to __bases__

6f06c52

Update missing docs

689267a

AlexWaygood reviewed Apr 9, 2023

View reviewed changes

Doc/library/types.rst Outdated Show resolved Hide resolved

Lib/test/test_types.py Outdated Show resolved Hide resolved

Lib/types.py Outdated Show resolved Hide resolved

Gobot1234 and others added 2 commits April 9, 2023 23:12

Apply suggestions from code review

1ae16ea

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Update Doc/library/types.rst

3a8619a

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood approved these changes Apr 9, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Apr 9, 2023

AlexWaygood self-assigned this Apr 9, 2023

AlexWaygood added 2 commits April 11, 2023 12:22

Merge remote-tracking branch 'upstream/main' into orig_class-and-bases

e1d55d6

Small tweaks to docs

bab8cb3

gvanrossum requested changes Apr 11, 2023

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Apr 11, 2023

Use assert in docs examples

fb9ef70

Document type.__orig_bases__

dc433c4

AlexWaygood reviewed Apr 12, 2023

View reviewed changes

Doc/reference/datamodel.rst Outdated Show resolved Hide resolved

Revert "Document type.__orig_bases__"

4268b74

This reverts commit dc433c4.

gh-101688: Implement types.get_original_bases #101827

gh-101688: Implement types.get_original_bases #101827

Gobot1234 commented Feb 11, 2023 •

edited by bedevere-bot

sobolevn left a comment

Gobot1234 commented Feb 18, 2023

sobolevn commented Feb 18, 2023

AlexWaygood left a comment

AlexWaygood left a comment

gvanrossum commented Apr 11, 2023

Gobot1234 commented Apr 11, 2023 •

edited

gvanrossum Apr 11, 2023

Gobot1234 Apr 12, 2023

gvanrossum Apr 12, 2023

AlexWaygood Apr 12, 2023 •

edited

gvanrossum Apr 12, 2023

gvanrossum Apr 12, 2023

AlexWaygood Apr 12, 2023 •

edited

AlexWaygood Apr 12, 2023

gvanrossum Apr 12, 2023

AlexWaygood Apr 12, 2023

bedevere-bot commented Apr 11, 2023

AlexWaygood commented Apr 12, 2023 •

edited

gvanrossum commented Apr 12, 2023

AlexWaygood Apr 12, 2023 •

edited

Gobot1234 Apr 12, 2023

AlexWaygood Apr 12, 2023 •

edited

	existed prior to any modification by :meth:`~object.__mro_entries__`. This
	existed before :meth:`~object.__mro_entries__` caused them to change. This

-    Return the objects in the bases list of the class's definition as they
-    existed prior to any modification by :meth:`~object.__mro_entries__`. This
-    is useful for introspecting :ref:`Generics <user-defined-generics>`.
+    Return the tuple of objects originally given as the bases of *cls* before
+    the :meth:`~object.__mro_entries__` method has been called on any bases
+    (following the mechanisms laid out in :pep:`560`). This is useful for
+    introspecting :ref:`Generics <user-defined-generics>`.
+    For classes that have an ``__orig_bases__`` attribute, this
+    function simply returns the value of ``cls.__orig_bases__``.

gh-101688: Implement types.get_original_bases #101827

Are you sure you want to change the base?

gh-101688: Implement types.get_original_bases #101827

Conversation

Gobot1234 commented Feb 11, 2023 • edited by bedevere-bot

sobolevn left a comment

Choose a reason for hiding this comment

Gobot1234 commented Feb 18, 2023

sobolevn commented Feb 18, 2023

AlexWaygood left a comment

Choose a reason for hiding this comment

AlexWaygood left a comment

Choose a reason for hiding this comment

gvanrossum commented Apr 11, 2023

Gobot1234 commented Apr 11, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Apr 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Apr 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Apr 11, 2023

AlexWaygood commented Apr 12, 2023 • edited

gvanrossum commented Apr 12, 2023

AlexWaygood Apr 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexWaygood Apr 12, 2023 • edited

Choose a reason for hiding this comment

Gobot1234 commented Feb 11, 2023 •

edited by bedevere-bot

Gobot1234 commented Apr 11, 2023 •

edited

AlexWaygood Apr 12, 2023 •

edited

AlexWaygood Apr 12, 2023 •

edited

AlexWaygood commented Apr 12, 2023 •

edited

AlexWaygood Apr 12, 2023 •

edited

AlexWaygood Apr 12, 2023 •

edited