-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
ruff rule UP007: Use X | Y for type annotations #8334
Conversation
eb7c1d7
to
acd3116
Compare
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.
Spun up the PR in gitpod and the search and such worked as expected. Also all the changes look correct to me. Just one question about openlibrary/solr/solr_types.py
.
@@ -175,10 +175,8 @@ max-statements = 70 | |||
"openlibrary/plugins/upstream/borrow.py" = ["BLE001", "E722"] | |||
"openlibrary/plugins/upstream/models.py" = ["BLE001"] | |||
"openlibrary/plugins/upstream/utils.py" = ["BLE001"] | |||
"openlibrary/solr/*" = ["UP007"] | |||
"openlibrary/solr/solr_types.py" = ["UP007"] |
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.
What stops us from migrating openlibrary/solr/solr_types.py
as well?
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.
Nothing ! That file is auto-generated though, so we'd need to change the file that generates it ( types_generator.py
)
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.
And the two # noqa: UP007
in scripts/solr_builder/tests/test_fn_to_cli.py
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! I believe cython/solr pipelines support this syntax , but if they don't it'll only cause an issue during full reindex, and I can easily address that.
Closes #8327
https://docs.astral.sh/ruff/rules/non-pep604-annotation
%
ruff --select=UP007 --fix .
%
ruff rule UP007
non-pep604-annotation (UP007)
Derived from the pyupgrade linter.
Autofix is sometimes available.
What it does
Check for type annotations that can be rewritten based on PEP 604 syntax.
Why is this bad?
PEP 604 introduced a new syntax for union type annotations based on the
|
operator. This syntax is more concise and readable than the previoustyping.Union
andtyping.Optional
syntaxes.This rule is enabled when targeting Python 3.10 or later (see:
[
target-version
]). By default, it's also enabled for earlier Pythonversions if
from __future__ import annotations
is present, as__future__
annotations are not evaluated at runtime. If your code relieson runtime type annotations (either directly or via a library like
Pydantic), you can disable this behavior for Python versions prior to 3.10
by setting [
pyupgrade.keep-runtime-typing
] totrue
.Example
Use instead:
Options
target-version
pyupgrade.keep-runtime-typing
Technical
Testing
Screenshot
Stakeholders