NotificationsYou must be signed in to change notification settings
Fork33.7k
Star70.4k

bpo-39481: Implementation for PEP 585#18239

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

gvanrossum merged 78 commits intopython:masterfromgvanrossum:pep585

Apr 7, 2020

Merged

bpo-39481: Implementation for PEP 585#18239

gvanrossum merged 78 commits intopython:masterfromgvanrossum:pep585

Apr 7, 2020

Conversation

Copy link

Member

gvanrossum commentedJan 28, 2020•
edited
Loading

I consider this a finished prototype now. We've got everything working, and then some (e.g. pickle), and while we found a fair number of minor issues with PEP 585, the main conclusion is that the PEP is quite reasonable to implement.

A number of suggestions for changes PEP 585 that have basically been vetted by@ambv can be found inpython/peps#1289.

There are a few places where I still disagree with@ambv. In particular, I don't think it makes sense to even allowisinstance(x, list[int]) andissubclass(C, list[int]) -- IMO these should just raise TypeError, and that's what this PR implements. (If@ambv convinces me to change my mind, it's easy to revert that.) [UPDATE:@ambv agrees and hasupdated the PEP.]

There are also a few things that ought to be changed:

We should add a nice docstring toGenericAlias.
We madeio.IOBase generic. This was a misunderstanding, and we'll revert that.
The__parameters__ attribute can and probably should be computed lazily the first time it's needed.
Maybe we still need a cache, liketyping.py uses?
Where should the code live? For now I put it indescrobject.c, but that's debatable (it was just convenient).

https://bugs.python.org/issue39481

gvanrossum requested a review fromserhiy-storchaka

January 28, 2020 15:41

gvanrossum requested review frommethane andrhettinger ascode owners

January 28, 2020 15:41

the-knights-who-say-ni added the CLA signed label

Jan 28, 2020

bedevere-bot added the awaiting core review label

Jan 28, 2020

gvanrossum removed the request for review fromrhettinger

January 28, 2020 15:42

serhiy-storchaka reviewed

Jan 28, 2020

View reviewed changes

Objects/descrobject.c OutdatedShow resolvedHide resolved

Objects/descrobject.c Outdated

		};

		staticPyMemberDefga_members[]= {
		{"__origin__",T_OBJECT, offsetof(gaobject,origin),READONLY},

Copy link

Member

serhiy-storchakaJan 28, 2020

Choose a reason for hiding this comment

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

MaybeT_OBJECT_EX?

Copy link

MemberAuthor

gvanrossumJan 28, 2020

Choose a reason for hiding this comment

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

I think not. The two attributes should never be allowed to be NULL. (This is a change from an earlier design.)

Objects/descrobject.c OutdatedShow resolvedHide resolved

Objects/descrobject.c Outdated

		staticPyObject*
		ga_new(PyTypeObjecttype,PyObjectargs,PyObject*kwds)
		{
		if (kwds!=NULL) {

Copy link

Member

serhiy-storchakaJan 28, 2020

Choose a reason for hiding this comment

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

It is worth to use Argument Clinic which generates simpler code using non-public C API similar to the following:

if (!_PyArg_NoKeywords("GenericAlias",kwds)||    !_PyArg_CheckPositional("GenericAlias",PyTuple_GET_SIZE(args),2,2)){returnNULL;}

Objects/descrobject.c OutdatedShow resolvedHide resolved

Objects/descrobject.c Outdated

		};

		staticPyObject*
		ga_new(PyTypeObjecttype,PyObjectargs,PyObject*kwds)

Copy link

Member

serhiy-storchakaJan 28, 2020

Choose a reason for hiding this comment

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

Do we need a__new__ method? Why not settp_new = NULL?

Copy link

MemberAuthor

gvanrossumJan 28, 2020

Choose a reason for hiding this comment

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

I first had a tp_init. Then calling GenericAlias() somehow worked and produced an object with NULL attributes (or maybe it was some more clever incantation, I forget, but I definitely saw it). I do want the class instantiable from Python (so it can potentially be used from typing.py). Using tp_new instead solved that problem.

gvanrossum changed the title~~WIP: PEP 585~~bpo-39481: WIP: PEP 585

Jan 28, 2020

gvanrossum added the DO-NOT-MERGE label

Jan 28, 2020

Copy link

MemberAuthor

gvanrossum commentedJan 29, 2020•
edited
Loading

So I'm looking into the test failures. I've fixed two simple ones, but now I'm stuck with at least two:

test_typing.py
test_genericalias.py (i.e. the very PEP 585 test) crashes on Ubuntu, but not for me locally , see below (UPDATE: fixed)

The problem with test_typing.py is that when we have a class that derives fromTuple[xxx], like so:

from typing import Tupleclass C(Tuple[int]): passprint(C.mro())  # C, tuple, typing.Generic, object

its MRO containstuplebeforeGeneric, and the newtuple.__class_getitem__ shadows the intendedGeneric.__class_getitem__. I'm not sure what to do about this.@ambv @ilevkivskyi @serhiy-storchaka

Copy link

MemberAuthor

gvanrossum commentedJan 29, 2020

Using a debug build I can repro the test_genericalias.py crash. The upper few lines of the stack trace are:

* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT  * frame #0: 0x00007fff72e217fa libsystem_kernel.dylib`__pthread_kill + 10    frame #1: 0x00007fff72edebc1 libsystem_pthread.dylib`pthread_kill + 432    frame #2: 0x00007fff72da8a1c libsystem_c.dylib`abort + 120    frame #3: 0x00007fff72da7cd6 libsystem_c.dylib`__assert_rtn + 314    frame #4: 0x00000001000df936 python.exe`_PyType_Lookup(type=0x0000000100401d30, name=0x00000001035f1d60) at typeobject.c:3155:5    frame #5: 0x00000001000c0b8f python.exe`_PyObject_GenericGetAttrWithDict(obj=0x0000000100401ec8, name=0x00000001035f1d60, dict=0x0000000000000000, suppress=0) at object.c:1319:13    frame #6: 0x00000001000c0ab3 python.exe`PyObject_GenericGetAttr(obj=0x0000000100401ec8, name=0x00000001035f1d60) at object.c:1407:12    frame #7: 0x00000001000c0268 python.exe`PyObject_GetAttr(v=0x0000000100401ec8, name=0x00000001035f1d60) at object.c:1013:16    frame #8: 0x00000001000c01a4 python.exe`PyObject_GetAttrString(v=0x0000000100401ec8, name="__module__") at object.c:918:11    frame #9: 0x0000000100056154 python.exe`ga_repr_item(writer=0x00007ffeefbe9078, p=0x0000000100401ec8) at descrobject.c:1828:24    frame #10: 0x0000000100054015 python.exe`ga_repr(self=0x00000001040349b0) at descrobject.c:1890:13    frame #11: 0x00000001000beefb python.exe`PyObject_Repr(v=0x00000001040349b0) at object.c:543:11    frame #12: 0x00000001001e9d49 python.exe`builtin_repr(module=0x0000000101221dd0, obj=0x00000001040349b0) at bltinmodule.c:2141:12

@ethanhs does this give you a clue? Methinks there's a refcount bug in ga_repr(). :-(

Copy link

MemberAuthor

gvanrossum commentedJan 29, 2020

@ethanhs Update: it's crashing inrepr(tuple[int, ...]). I think I know the problem. Fix coming up.

Copy link

MemberAuthor

gvanrossum commentedJan 29, 2020

Okay, so test_typing.py is the only failing test (see above). But I think the gc_repr() code might be refactored a bit to avoid getting the__origin__ and__parameters__ attributes before we know they are needed.

Copy link

Member

emmatyping commentedJan 29, 2020•
edited
Loading

But I think the gc_repr() code might be refactored a bit to avoid getting theorigin andparameters attributes before we know they are needed.

Ah, are you suggesting that we move those into the branch where we check fro__qualname__ and__module__?

(also thank you for fixing the crash!)

serhiy-storchaka reviewed

Jan 29, 2020

View reviewed changes

Lib/test/test_genericalias.py Outdated

Copy link

Member

serhiy-storchakaJan 29, 2020

Choose a reason for hiding this comment

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

TODO: I suppose IOBase subclasses BufferedIOBase, RawIOBase and TextIOBase should not support indexing.

Copy link

Member

serhiy-storchakaJan 29, 2020

Choose a reason for hiding this comment

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

There is also Python implementation_pyio.IOBase. I am not sure what to do with it. Maybe just ignore for now.

Copy link

MemberAuthor

gvanrossumJan 29, 2020•
edited
Loading

Choose a reason for hiding this comment

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

I added__class_getitem__ to_pyio.IOBase in 35cd5f477c9da985880130d72297335e010450e4.

I'm not sure whether it matters much that we take the__class_getitem__ away from the various subclasses (let the static type checkers worry about that).

gvanrossum requested a review from1st1 as acode owner

January 30, 2020 00:05

gvanrossumand others added14 commits

January 29, 2020 16:29

PEP 585 step 2: Write a Py_GenericAlias() function that returns origin

d557a64

PEP 585 step 3: Add Py_GenericAlias() as __class_getitem__ to tuple, …

da33267

…list, dict, set, frozenset

PEP 585 step 4: Write some tests

447fbe9

Define a GenericAlias (proxy) object (with lots of TODOs still)

83e2ba2

More tests

fb3e8d3

PEP 585 steps 5-6: Finish basic GenericAlias object and test

235180b

Fix bug with class methods

70f3abc

Define tp_new instead of tp_init

e4d1356

Test that list[int][int] fails

8f7a55e

Test that class L(list): pass; L[int] works

f2ba2e5

Implement PEP 585 for collections.deque and ... (#2)

0af74d1

... collections.defaultdict

Respond to Serhiy's code review

b264294

📜🤖 Added by blurb_it.

0ddea56

PEP 585 steps 7-8: Implement repr for GenericAlias (#1)

91edf1c

By Ethan Smith.

gvanrossumand others added2 commits

April 6, 2020 12:01

Use PyObject_Str(qualname)

95b3c2a

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>

Merge remote-tracking branch 'upstream/master' into pep585

13d3742

Copy link

MemberAuthor

gvanrossum commentedApr 6, 2020

What's up with the Docs build failures in CI?

Delete mention of pep585 branch in GitHub workflow

d3839ea

Copy link

MemberAuthor

gvanrossum commentedApr 6, 2020

Can't say I understand the Windows failures either. Anyway, I'm prepping this for merge by deleting the GitHub workflow edit. I'll be back later.

Copy link

MemberAuthor

gvanrossum commentedApr 6, 2020

Hey@ethanhs -- can you repro the Windows failure here on test_ctypes?

Copy link

Member

emmatyping commentedApr 6, 2020

@gvanrossum I cannot repro locally. It seems to be unable to find the sqlite shared library, but I have no idea how that could be caused by this changeset.

Copy link

MemberAuthor

gvanrossum commentedApr 6, 2020

Thanks -- I'll just land.

Copy link

MemberAuthor

gvanrossum commentedApr 6, 2020

Oops. "Required statuses must pass before merging"

gvanrossum removed the awaiting review label

Apr 6, 2020

gvanrossum closed this

Apr 6, 2020

gvanrossum reopened this

Apr 6, 2020

Move Py_GenericAlias* to genericaliasobject.h

31fd842

Copy link

Member

serhiy-storchaka commentedApr 7, 2020

Should not we addPy_GenericAlias and/orPy_GenericAliasType inPC/python3.def?

gvanrossum added2 commits

April 7, 2020 08:22

Merge remote-tracking branch 'upstream/master' into pep585

6bad315

Add Py_GenericAlias[Type] to PC/python3.ref

cdb46c3

Copy link

MemberAuthor

gvanrossum commentedApr 7, 2020

Should not we add Py_GenericAlias and/or Py_GenericAliasType in PC/python3.def?

Done, both. And the test failure is resolved after the merge from master, so once this passes I'll land.

gvanrossum merged commit48b069a intopython:master

Apr 7, 2020

Copy link

bedevere-bot commentedApr 7, 2020

@gvanrossum: Please replace# withGH- in the commit message next time. Thanks!

bedevere-bot removed the awaiting merge label

Apr 7, 2020

gvanrossum deleted the pep585 branch

April 7, 2020 16:50

BvB93 mentioned this pull request

May 7, 2021

API: Addnpt.NDArray, a runtime-subscriptable alias fornp.ndarraynumpy/numpy#18935

Merged

colesbury referenced this pull request in colesbury/nogil

Oct 6, 2021

bpo-39481: Implementation for PEP 585 (#18239)

6b60ad9

This implements things like `list[int]`,which returns an object of type `types.GenericAlias`.This object mostly acts as a proxy for `list`,but has attributes `__origin__` and `__args__`that allow recovering the parts (with values `list` and `(int,)`.There is also an approximate notion of type variables;e.g. `list[T]` has a `__parameters__` attribute equal to `(T,)`.Type variables are objects of type `typing.TypeVar`.

icanhasmath pushed a commit to ActiveState/cpython that referenced this pull request

Sep 27, 2023

bpo-39481: Implementation for PEP 585 (python#18239)

18682b3

This implements things like `list[int]`,which returns an object of type `types.GenericAlias`.This object mostly acts as a proxy for `list`,but has attributes `__origin__` and `__args__`that allow recovering the parts (with values `list` and `(int,)`.There is also an approximate notion of type variables;e.g. `list[T]` has a `__parameters__` attribute equal to `(T,)`.Type variables are objects of type `typing.TypeVar`.

icanhasmath pushed a commit to ActiveState/cpython that referenced this pull request

Sep 29, 2023

bpo-39481: Implementation for PEP 585 (python#18239)

1a7e7fd

This implements things like `list[int]`,which returns an object of type `types.GenericAlias`.This object mostly acts as a proxy for `list`,but has attributes `__origin__` and `__args__`that allow recovering the parts (with values `list` and `(int,)`.There is also an approximate notion of type variables;e.g. `list[T]` has a `__parameters__` attribute equal to `(T,)`.Type variables are objects of type `typing.TypeVar`.