GeezerVison is showing github.com — GH-118761: Expose more core interpreter types in ``_types`` by AA-Turner · Pull Request #132103 · python/cpython
Conversation
AA-Turner
commented
Apr 5, 2025
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
This was discussed in #131969 (cc @ZeroIntensity), but deferred to get the smaller change in. This PR adds all of the core interpreter types currently exposed in the types module to _typesmodule.c. We then change the definition of types.py to prefer these, but if the _types module is not available, to use the pure-Python fallbacks.
Strictly, this isn't needed, as _types is a built-in module. I think it might be helpful to keep the pure Python definitions for other implementations (PyPy, etc), and perhaps as a quick reminder of what each type looks like. Open to thoughts on this, as it would clearly be simpler to replace everything with from _types import *.
I've initially categorised this under #118761, as this does provide a ~15% speed-up for me (1627µs to 1403µs).
A
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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 might be worth adding a _pytypes module to hold the pure-Python fallbacks.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
It might be worth adding a _pytypes module to hold the pure-Python fallbacks.
As in the following?
try:
from _types import *
except ImportError:
from _pytypes import *
Could you elaborate on the benefits?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Could you elaborate on the benefits?
One benefit is that it reduces by one the indentation level and it's easier to read/modify I think. Also, we do it for _pydecimal so why not doing it for _pytypes?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Could you elaborate on the benefits?
Yeah, what Bénédikt said. It seems cleaner to encapsulate each implementation.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Also, we do it for _pydecimal so why not doing it for _pytypes?
_pydecimal, _pyio, etc are Python implementations of the C accelarator modules (see PEP-0399), whereas types still contains quite a lot only implemented in Python. See e.g. collections and functools which use extension modules, but don't have a _py<mod> helper module.
A
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I don't have much of a preference, it's just prettier. Are we planning to implement the other functions in C?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Are we planning to implement the other functions in C?
I'm not personally, though Yury left a TODO comment by _GeneratorWrapper about implementing it in C. I'd probably prefer to keep the simpler functions in Python, but there might be benefits I haven't considered to converting to C.
A
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
LGTM. Let's do a _pytypes module in a follow-up if we decide to implement the rest of the functions in C.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Just some cosmetic suggesetions. Concerning the spaces before the comments, I don't think we now need to have some column-aligned comments (it's not even clear that they are aligned in the first place).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
picnixz
picnixz approved these changes
+1 more reviewer
Reviewers whose approvals may not affect merge requirements
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|