Skip to content

gh-148406: Fix annotations of _colorize.FancyCompleter#148408

Open
danielhollas wants to merge 1 commit intopython:mainfrom
danielhollas:fix-fancy-completer
Open

gh-148406: Fix annotations of _colorize.FancyCompleter#148408
danielhollas wants to merge 1 commit intopython:mainfrom
danielhollas:fix-fancy-completer

Conversation

@danielhollas
Copy link
Copy Markdown
Contributor

@danielhollas danielhollas commented Apr 11, 2026

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).

@danielhollas
Copy link
Copy Markdown
Contributor Author

This is a change in a private module that should not be user facing so I don't think blurb is needed?

@@ -1,3 +1,4 @@
import builtins
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just do from builtins import str as _str? it may be a bit easier to read then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we could, but I kind of like the explicitness of the current solution. But if others agree I am happy to change it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm also good with that. I just know that in other modules we also have _str = builtins.str or _int = builtins.int. I'll let Hugo decide (I just find the repetition of builtins.str a bit less readable but that's my opinion)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, if it is already a pattern elsewhere then I am more okay with that. Will wait for others to chime in before changing.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Annotations of _colorize.FancyCompleter dataclass are funky

3 participants