-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
PEP 604 NewType #88519
Comments
|
It is possible to implement NewType to return a class rather than a function as it does currently. However, the class version uses substantially more memory (1072 bytes vs 144 bytes with sys.getsizeof) and NewType is supposed to have almost no runtime overhead. |
What about to return callable object instead of function from a It will look something like this:
With such implementation |
python/typing#746 has some previous discussion of implementing NewType as a class (motivated by __repr__), including benchmarks. |
Jelle I can try to implement it in C to see if it will give better performance. |
@yurii, I would like to caution against a C accelerator for the typing module. My reasoning follows:
Unfortunately, I can't offer a good solution to this issue at the moment either. |
Do we have to solve it? Using |
Not in type aliases or generic bases. (And if Larry's co_annotations PEP is accepted, it would break in normal annotations too.) I'd prefer to get rid of this sort of subtle difference where the obvious way to write a type only works in some contexts, though I realize that's not possible in all cases. That means I think we should move forward with making NewTypes into instances with __call__ instead of functions, though according to my benchmarks in the typing issue that will incur a small performance penalty. |
I opened PR with refactored Also I have added |
Jelle, can you post more details about your benchmark? |
This is what I got last year (copied from the typing issues): In [83]: from typing import NewType In [84]: nt = NewType("nt", int) In [85]: class NewTypeClass: In [86]: ntc = NewTypeClass("ntc", int) In [87]: ntc In [88]: %timeit nt(3) In [89]: %timeit ntc(3) Not sure what version of Python I used at the time. I just repeated it on 3.9.4 MacOS and got much faster time but a similar ~20% slowdown: In [6]: %timeit nt(3) In [7]: %timeit ntc(3) |
Hm, 20% isn't so bad, but one of the arguments for NewType was that it "disappears" at runtime -- which the current version does, but the class-based version doesn't quite. Right now I don't have the cycles to think about this deeply. Maybe a few weeks after I'm back from vacation (~August 8) I will have more time. |
This issue is now out of date on. After Serhiy's refactoring, any function types can be unioned. >>> NewType('x', int)
>>> int | NewType('x', int)
int | typing.NewType.<locals>.new_type The only problem now is that the repr is weird, but Serhiy also has a fix for that in bpo-34963 (PR 9951) without converting to a class. Thus, I am closing the issue. Please go to bpo-44642 for a general discussion on whether union should support arbitrary functions at all, or bpo-34963 for the repr problem. Thanks all! |
Does that work if you try to union two NewTypes? |
Oh woops I was too eager in closing this issue -- it doesn't work. Sorry, please disregard my previous message. I'm reopening this. Thanks for the reminder Jelle. |
NewType was described originally in python/typing#189 and the resulting PEP text landed in https://github.com/python/typing/pull/226/files. I find the 20% performance impact non-ideal but ultimately worth it because:
Since we can't fully rely on Currently adoption of NewType is relatively low, in part due to the feature's obscurity and partially because of its limits. Slower instantiation performance will at worst keep the adoption low, but it can potentially bring more users to NewType since it becomes less hacky. I'm +1 to converting to a class as done by the PR.
Sorry, I overeagerly already merged the change to |
OTOH, for 3.10 the clock is ticking. Let's go for it. (If it's deemed too slow, we could eventually add an accelerator just for this.) |
I found that replacing __call__ on the NewType class with an identity function written in C makes things faster instead: In [54]: %timeit ntc2(1) In [55]: %timeit ntc(1) In [56]: %timeit nt(1) Here ntc2 has __call__ implemented in C, ntc is the previous class-based version and nt is the current function-based version. So perhaps we could just stick a private |
Jelle thanks for pointing this out, I will implement helper function in C and use it as a __call__ method for NewType |
Note: we won't be backporting _typing to Python 3.10 as it is much too late for a new module at this point in the life cycle. Consequently, 3.10 will be a (temporary) performance regression in terms of typing.NewType. Thanks everyone, most of all Josepth for reporting and Yurii for implementing the required changes. |
PR27262 introduced reference leaks and currently all refleak buildbots are red: Example: https://buildbot.python.org/all/#/builders/205/builds/102/steps/5/logs/stdio Ran 367 tests in 0.198s |
The buildbots are still not green. Unfortunately we will need to revert all PRs if this is not fixed soon as this is already making other issues |
@pablo, I don't think this change is causing the buildbots to fail. The test failure on all the currently failing buildbots is: ====================================================================== Traceback (most recent call last):
File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.11/test/test_import/__init__.py", line 1355, in test_absolute_circular_submodule
import test.test_import.data.circular_imports.subpkg2.parent
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: No module named 'test.test_import.data.circular_imports.subpkg2' Which seems to be #71486. |
Pick any of the failing refleak buildbots. For example: https://buildbot.python.org/all/#/builders/280/builds/99 You will see: ---------------------------------------------------------------------- |
types is not typing. This is a different failure from what we've seen before. I'll fix that today. |
Bisecting points to: fe13f0b is the first bad commit
Lib/test/test_types.py | 35 +++++++++++++++ |
I am closing this one again, apologies for the confusion. |
Thanks a bunch Yurii, Serhiy, Jelle, Łukasz and Pablo for working on this! I'm re-closing this issue. *Fingers-crossed* we won't have to open this again ;-). |
NewType
#29785NewType
(GH-29785) #29796Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: