Skip to content
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.

Sign up for GitHub

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

Improve types in utils.termcolors #1901

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Improve types in utils.termcolors #1901

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abece73
Improve typings in `utils/termcolors/pyi`
Viicos Jan 6, 2024
08c563f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 6, 2024
b1fee9b
RESET: Literal["0"]
Viicos Jan 13, 2024
c623884
Add allowlist entry
Viicos Jan 14, 2024
cfbf278
Fix tuple type
Viicos Mar 20, 2024
c9b49a6
Fix allowlist
Viicos Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions django-stubs/utils/termcolors.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
from collections.abc import Callable, Mapping, Sequence
from typing import Any
from collections.abc import Callable, Sequence
from typing import Any, Literal

color_names: Sequence
foreground: Mapping[str, str]
background: Mapping[str, str]
RESET: str
opt_dict: Mapping[str, str]
color_names: tuple[str]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget, if it's a tuple with multiple elements of the same type, you need .... This should fix the allowlist entry too.

Suggested change
color_names: tuple[str]
color_names: tuple[str, ...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, inattention from my end

foreground: dict[str, str]
background: dict[str, str]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The available colors seem to be fixed, should we switch to:

Suggested change
color_names: tuple[str]
foreground: dict[str, str]
background: dict[str, str]
color_names: Final[tuple["black", "red", "green", "yellow", "blue", "magenta", "cyan", "white"]]
foreground: # Some kind of typed dict
background: # Same

Copy link
Member

Choose a reason for hiding this comment

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

Why Final? I think it should be tuple[Literal[...], ...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that users don't actually set these variables to something else. But it's fine without it as well, I feel Final conveys a stronger meaning (e.g. with numpy and np.PI).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ended up keeping tuple[str], literals feel a bit overkill

RESET: Literal["0"]
opt_dict: dict[str, str]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
opt_dict: dict[str, str]
opt_dict: # A typed dict?


def colorize(text: str | None = ..., opts: Sequence[str] = ..., **kwargs: Any) -> str: ...
def make_style(opts: tuple = ..., **kwargs: Any) -> Callable: ...
def colorize(text: str | None = ..., opts: Sequence[str] = ..., *, fg: str = ..., bg: str = ...) -> str: ...
def make_style(opts: Sequence[str] = ..., *, fg: str = ..., bg: str = ...) -> Callable[[str | None], str]: ...
Viicos marked this conversation as resolved.
Show resolved Hide resolved

NOCOLOR_PALETTE: str
DARK_PALETTE: str
Expand Down
3 changes: 3 additions & 0 deletions scripts/stubtest/allowlist.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,3 +390,6 @@ django.urls.resolvers.URLPattern.lookup_str
django.urls.resolvers.URLResolver.url_patterns
django.urls.resolvers.URLResolver.urlconf_module
django.utils.connection.BaseConnectionHandler.settings

# Literals
django.utils.termcolors.color_names