classification
Title: Implement PEP 560: Core support for typing module and generic types
Type: enhancement Stage: resolved
Components: Interpreter Core, Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: levkivskyi Nosy List: gvanrossum, levkivskyi, ned.deily, serhiy.storchaka, xgdomingo
Priority: high Keywords: patch

Created on 2017-12-05 20:36 by levkivskyi, last changed 2018-01-29 23:35 by levkivskyi. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4732 merged levkivskyi, 2017-12-05 20:48
PR 4906 merged levkivskyi, 2017-12-16 21:47
PR 5098 merged serhiy.storchaka, 2018-01-04 13:08
PR 5212 merged vstinner, 2018-01-17 10:28
Messages (28)
msg307682 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-12-05 20:38
As discussed before, there will be two PRs. One for the core components, and the second one (large) for typing updates. I will open the first PR shortly.
msg308092 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-12 08:21
As for __class_getitem__, why not implement type.__getitem__ instead of hacking PyObject_GetItem()?
msg308101 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-12 09:49
For reference: PEP 560.
msg308121 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-12-12 12:23
> As for __class_getitem__, why not implement type.__getitem__ instead of hacking PyObject_GetItem()?

This question was raised by Mark Shannon on python-dev. Actually, my initial implementation did exactly this, but I didn't like it for two reasons:

1. dir(type) is already very long, I don't want to make it even longer.
2. Some users may be surprised that although ``type.__getitem__`` is defined, ``int[something]`` will rise "TypeError: type object is not subscriptable"

Mark however disappeared since then, so I don't know what is his current opinion. I am however not as sure about this now. If there are some arguments for this, then I can revert to the older approach (implementing ``type.__getitem__``).
msg308151 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-12-12 21:10
Guido, what is your preference for the implementation of ``__class_getitem__``: ``PyObject_GetItem`` (current one) or ``type.__getitem__`` (old one)? Can we just go ahead with the version you like and then re-consider if some objections will appear? I am asking because you are going on vacation soon and there is a second part of implementation -- changes in typing (and someone needs to review them :-)
msg308171 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-12-12 23:37
I like the current approach (PyObject_GetItem).  I agree with both of your reasons to prefer it.
msg308291 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-14 12:22
This adds an overhead to every indexing.

$ ./python -m perf timeit --compare-to=./python0 -s 'a = [1]' --duplicate=10000 'a[0]'
python0: ..................... 15.3 ns +- 0.5 ns
python: ..................... 16.1 ns +- 0.3 ns

Mean +- std dev: [python0] 15.3 ns +- 0.5 ns -> [python] 16.1 ns +- 0.3 ns: 1.05x slower (+5%)

$ ./python -m perf timeit --compare-to=./python0 -s 'd = {1: 2}' --duplicate=10000 'd[1]'
python0: ..................... 17.6 ns +- 0.6 ns
python: ..................... 18.4 ns +- 0.5 ns

Mean +- std dev: [python0] 17.6 ns +- 0.6 ns -> [python] 18.4 ns +- 0.5 ns: 1.05x slower (+5%)
msg308312 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-12-14 16:23
> This adds an overhead to every indexing.

I'm not doubting your results, but I'm curious how that happened. The extra
code in PyObject_GetItem is right before the point where it would otherwise
raise TypeError -- I presume you benchmark never reached that point. Could
it be that the compiler ends up rearranging the code in a way that is less
optimal for the instruction cache?
msg308323 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-14 17:25
I think this is a cause. The difference may be dependent on the compiler and platform.
msg308341 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-12-14 22:33
New changeset 2b5fd1e9ca9318673989e6ccac2c8acadc3809cd by Ivan Levkivskyi in branch 'master':
bpo-32226: Implementation of PEP 560 (core components) (#4732)
https://github.com/python/cpython/commit/2b5fd1e9ca9318673989e6ccac2c8acadc3809cd
msg308346 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2017-12-14 23:43
Interesting. I have noticed similar when I tried to add a fast loop proposed by Serhiy. It should save at least one pointer comparison for base class, but sometimes it actually led to slow-downs (although very small). How can one fight this kind of problems?
msg308347 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2017-12-14 23:46
I think the best thing to do is not to panic! Also maybe PGO could help?

On Dec 14, 2017 15:43, "Ivan Levkivskyi" <report@bugs.python.org> wrote:

>
> Ivan Levkivskyi <levkivskyi@gmail.com> added the comment:
>
> Interesting. I have noticed similar when I tried to add a fast loop
> proposed by Serhiy. It should save at least one pointer comparison for base
> class, but sometimes it actually led to slow-downs (although very small).
> How can one fight this kind of problems?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue32226>
> _______________________________________
>
msg308351 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-12-15 00:12
It is very to get stable benchmarks with timings shorter than 1 ms, and even harder for timings shorter than 1 us.

I wrote documentation to implement how to get more stable results:
http://perf.readthedocs.io/en/latest/run_benchmark.html#how-to-get-reproductible-benchmark-results

On Linux, using CPU isolation helps a lot, and perf uses automatically isolated CPUs.
msg309484 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-04 22:21
New changeset ce5b0e9db1b9698e6ffc43ae41cf3a22ca5a6ba6 by Serhiy Storchaka in branch 'master':
bpo-32226: Make __class_getitem__ an automatic class method. (#5098)
https://github.com/python/cpython/commit/ce5b0e9db1b9698e6ffc43ae41cf3a22ca5a6ba6
msg309485 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-04 22:24
Please don't forgot to document this feature.
msg309829 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-11 20:29
Can I help?

I'm implementing the feature related to pickling classes, and want to implement pickling generic types as an example. But since PEP 560 will change all here, I need to wait this change.
msg309830 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-01-11 22:36
Serhiy,

I am sorry for a delay, I have recently moved to another country, this is why I have not much time. I will try to work on this weekend. Here are some points where you can be helpful:

* Make a short documentation PR for `__mro_entry__` and `__base_subclass__` methods.
* Document the `__class_getitem__` C API calling convention (with motivation, many classes are implemented in C but are generic in nature, like numpy.ndarray) in PEP 560.

(I know this might be not very interesting, but this will save time for me to focus only on typing part.)
msg309927 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-14 16:36
Sorry, I know very small about this module to writing the documentation.
msg310149 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-17 10:23
Since the commit 45700fb757591a672e9d25b8252971c2a2caeaf2, test_genericclass leaks references:
---
vstinner@apu$ ./python -m test -R 3:3 test_genericclass -m test.test_genericclass.CAPITest.test_c_class
Run tests sequentially
0:00:00 load avg: 1.27 [1/1] test_genericclass
beginning 6 repetitions
123456
......
test_genericclass leaked [2, 3, 2] memory blocks, sum=7
test_genericclass failed

1 test failed:
    test_genericclass

Total duration: 107 ms
Tests result: FAILURE
---

The leak comes from _testcapi.Generic[int]. Maybe from generic_alias_new() of Modules/_testcapi.c.
msg310150 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-17 10:29
Oh ok, I think that I found the bug: PR 5212.
msg310208 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-17 22:08
New changeset e860089fd93728824cf586fa4d91f08eff3bab73 by Victor Stinner in branch 'master':
bpo-32226: Fix memory leak in generic_alias_dealloc() (#5212)
https://github.com/python/cpython/commit/e860089fd93728824cf586fa4d91f08eff3bab73
msg310336 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-01-20 11:24
New changeset d911e40e788fb679723d78b6ea11cabf46caed5a by Ivan Levkivskyi in branch 'master':
bpo-32226: PEP 560: improve typing module (#4906)
https://github.com/python/cpython/commit/d911e40e788fb679723d78b6ea11cabf46caed5a
msg311027 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-28 22:51
What's the status of this?  The issue is still open with "release blocker" status.
msg311035 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-01-29 00:15
I think all the critical things have been implemented/fixed. There are unfortunately no docs yet (my fault), also there are two bugs related to PEP 560, but they are not new, they also exist in Python 3.6.

I am however marking this as high priority, since these two bugs prevent progress on other projects (pickling generic classes), so it would be good to fix them before beta-2.
msg311036 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-29 00:21
Are there more specific descriptions of those bugs? Links to a tracker?
(Even if it's the typing tracker.)
msg311038 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-01-29 01:00
https://github.com/python/typing/issues/512 and https://github.com/python/typing/issues/511 are first issue (they are closely related to each other). This is not directly related to PEP 560, but changes in typing because of it will allow to fix them in a simple manner.

The second one is Union simplification (was discussed in few PRs on typing tracker). This is not really a bug, but something we should decide on before 3.7 final. It will be difficult to change it later. Again, with PEP 560 in place the corresponding refactoring for removal should be straightforward.

In addition, there is https://github.com/python/typing/issues/392, this is already almost fixed by the typing PR implementing PEP 560, but it doesn't work for dunder attributes (I think we should not relay only specific set of reserved names, rather than all dunders).
msg311148 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2018-01-29 16:27
ISTM that those ought to be separate issues since PEP 560 has been
implemented fully here.

Also I am getting cold feet about Union simplification (esp. this late in
the release cycle). We should probably retreat on the typing tracker and
discuss why we're considering it -- is there evidence (even anecdotal) that
it's harmful?
msg311199 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2018-01-29 23:35
OK, I will close this issue, and open separate issues for documentation, Union, etc.
History
Date User Action Args
2018-01-29 23:35:04levkivskyisetstatus: open -> closed
resolution: fixed
messages: + msg311199

stage: patch review -> resolved
2018-01-29 16:27:55gvanrossumsetmessages: + msg311148
2018-01-29 01:00:11levkivskyisetmessages: + msg311038
2018-01-29 00:21:14gvanrossumsetmessages: + msg311036
2018-01-29 00:15:23levkivskyisetpriority: release blocker -> high

messages: + msg311035
2018-01-28 22:54:05vstinnersetnosy: - vstinner
2018-01-28 22:51:15ned.deilysetmessages: + msg311027
2018-01-20 11:24:03levkivskyisetmessages: + msg310336
2018-01-17 22:08:21vstinnersetmessages: + msg310208
2018-01-17 10:29:11vstinnersetmessages: + msg310150
2018-01-17 10:28:39vstinnersetpull_requests: + pull_request5066
2018-01-17 10:23:03vstinnersetmessages: + msg310149
2018-01-14 16:36:18serhiy.storchakasetmessages: + msg309927
2018-01-11 22:36:46levkivskyisetmessages: + msg309830
2018-01-11 20:29:46serhiy.storchakasetmessages: + msg309829
2018-01-04 22:24:23serhiy.storchakasetmessages: + msg309485
2018-01-04 22:22:39serhiy.storchakasetpriority: normal -> release blocker
nosy: + ned.deily
2018-01-04 22:21:49serhiy.storchakasetmessages: + msg309484
2018-01-04 13:08:07serhiy.storchakasetpull_requests: + pull_request4966
2017-12-16 21:47:46levkivskyisetpull_requests: + pull_request4800
2017-12-15 00:12:29vstinnersetmessages: + msg308351
2017-12-14 23:46:37gvanrossumsetmessages: + msg308347
2017-12-14 23:43:25levkivskyisetmessages: + msg308346
2017-12-14 22:33:03levkivskyisetmessages: + msg308341
2017-12-14 17:25:09serhiy.storchakasetnosy: + vstinner
messages: + msg308323
2017-12-14 16:23:21gvanrossumsetmessages: + msg308312
2017-12-14 12:22:24serhiy.storchakasetmessages: + msg308291
2017-12-13 03:57:22xgdomingosetnosy: + xgdomingo
2017-12-12 23:37:15gvanrossumsetmessages: + msg308171
2017-12-12 21:10:56levkivskyisetmessages: + msg308151
2017-12-12 12:23:20levkivskyisetmessages: + msg308121
2017-12-12 09:49:21serhiy.storchakasetmessages: + msg308101
2017-12-12 08:21:54serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg308092
2017-12-05 20:48:27levkivskyisetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request4635
2017-12-05 20:38:12levkivskyisetnosy: + gvanrossum
messages: + msg307682
2017-12-05 20:36:48levkivskyicreate