-
Notifications
You must be signed in to change notification settings - Fork 970
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
Make build backend type annotations more generic #10549
Conversation
This PR makes the dictionary type annotations more generic with the `typing.Mapping` abstract class. Suggested in astral-sh#10434 (comment)
python/uv/_build_backend.py
Outdated
import sys | ||
|
||
if config_settings: | ||
print("Warning: Config settings are not supported", file=sys.stderr) | ||
|
||
|
||
def call(args: "list[str]", config_settings: "dict[Any, Any] | None" = None) -> str: | ||
def call(args: "list[str]", config_settings: "Mapping[Any, Any] | None" = None) -> str: |
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.
list
is invariant, so it list[str]
won't accept list[LiteralString]
or other lists whose value type is strict subtype of str
.
So if this wasn't intended, then you could use collections.abc.Sequence
, which has a covariant value type parameter.
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.
done in 7250bdb
python/uv/_build_backend.py
Outdated
@@ -18,17 +18,17 @@ | |||
|
|||
TYPE_CHECKING = False | |||
if TYPE_CHECKING: | |||
from typing import Any # noqa:I001 | |||
from typing import Any, Mapping # noqa:I001 |
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.
typing.Mapping
is deprecated since python 3.9, so it's recommended to import it from collections.abc
: https://docs.python.org/3/library/typing.html#typing.Mapping
And it apparently already works on Python 3.8:
https://basedpyright.com/?pythonVersion=3.8&typeCheckingMode=all&code=GYJw9gtgBAxmA28CmMAuBLMA7AzgOgEMAjGKdCABzBFSgFkCKL0sBzAWACguCAuexszYBtHKhAAaKGJABdKAF4oAbwC%2BXIA
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.
I also checked on PyPI, and the uv python package only supports py38+, so this is doable. Updated in 808eb2d.
ping @konstin :) |
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.
thanks for following up!
Summary
As a follow-up to #10434, this PR makes the dictionary type annotations more generic with the
typing.Mapping
abstract class.In theory, this allows
typing.TypedDict
to be passed. However, note that in practice, the uv build backend is set up to reject if anything is actually passed to this optional parameter 😄Suggested in #10434 (comment)
Test Plan
uvx ruff check --select ANN python/
checks typing is completeuvx mypy --strict python/
checks typing is consistent