GeezerVison is showing github.com — gh-148406: Fix annotations of _colorize.FancyCompleter by danielhollas · Pull Request #148408 · python/cpython
Conversation
Fixes #148406, using an approach suggested in the issue.
Test plan
Before
>>> import _colorize, pprint
>>> pprint.pprint(_colorize.FancyCompleter.__annotations__)
{'NoneType': '\x1b[1;32m',
'bool': '\x1b[1;32m',
'builtin_function_or_method': '\x1b[1;32m',
'bytes': '\x1b[1;32m',
'complex': '\x1b[1;32m',
'float': '\x1b[1;32m',
'function': '\x1b[1;32m',
'int': '\x1b[1;32m',
'method': '\x1b[1;32m',
'method_descriptor': '\x1b[1;32m',
'method_wrapper': '\x1b[1;32m',
'module': '\x1b[1;32m',
'str': '\x1b[1;32m',
'type': '\x1b[1;32m',
'wrapper_descriptor': '\x1b[1;32m'}
After
>>> from _colorize import FancyCompleter
>>> from pprint import pprint
>>> pprint(FancyCompleter.__annotations__)
{'NoneType': <class 'str'>,
'bool': <class 'str'>,
'builtin_function_or_method': <class 'str'>,
'bytes': <class 'str'>,
'complex': <class 'str'>,
'float': <class 'str'>,
'function': <class 'str'>,
'int': <class 'str'>,
'method': <class 'str'>,
'method_descriptor': <class 'str'>,
'method_wrapper': <class 'str'>,
'module': <class 'str'>,
'str': <class 'str'>,
'type': <class 'str'>,
'wrapper_descriptor': <class 'str'>}
Also added a regression tests to ensure that re module is not imported during _colorize import.
We've just made re import lazy in dataclasses (#148379), but because of the bug resolved here, it ended up being imported (because the annotations were strings, which was triggering the path that uses the re module).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This is a change in a private module that should not be user facing so I don't think blurb is needed?
|
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.
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.
One more idea: there's a special section for type imports in if False: right below. Maybe you should utilize it?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
One more idea: there's a special section for type imports in if False: right below. Maybe you should utilize it?
I think that's only worth doing if the import is slow (so for typing it makes sense), otherwise you're just making the runtime annotations worse which will technically make the dataclass construction ever so slightly slower as get_annotations now has to build a bunch of ForwardRef objects1.
Footnotes
-
I'm not sure if this would be measurable in context in _colorize ↩
|
AlexWaygood and danielhollas reacted with thumbs up emoji
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.
ljfp
pushed a commit
to ljfp/cpython
that referenced
this pull request
Apr 25, 2026
Reviewers
picnixz
picnixz left review comments
hugovk
hugovk approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
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.
|