[pyupgrade] Clarify fix safety docs (UP007, UP045)#25288
Merged
ntBre merged 2 commits intoMay 21, 2026
Conversation
The current "Fix safety" sentence ("may lead to runtime errors when
alongside libraries...") obscured which combination of [`target-version`]
and comments actually makes the fix unsafe, and the issue's filer hit
the rule fix on Python 3.10+ (where the fix is safe) and was confused
into thinking the docs implied a stronger warning.
Reword as a single rule + two reasons, in line with @pjonsson's
suggestion of flipping the sentence around: the fix is safe when target
is 3.10+ and there are no comments, otherwise unsafe because (a) pre-3.10
runtime introspection (Pydantic, etc.) can `TypeError` on `X | Y`, or
(b) inline comments would be dropped during rewrite. Applied symmetrically
to UP045's docstring since it had the same phrasing.
Closes astral-sh#17062.
|
for future reference, I think the "unusal and likely incorrect" part of this should probably be in a separate Known Issues section, as I don't think this is isolated to Python versions before 3.10, and the code doesn't actually check anything about the types to determine fix safety. astral-sh#9306 added this phrase, which I think is worth having somewhere, just not necessarily in this section
pyupgrade] Clarify UP007 / UP045 fix safety docspyupgrade] Clarify fix safety docs (UP007, UP045)
thejchap
pushed a commit
to thejchap/ruff
that referenced
this pull request
May 23, 2026
…5288) The current UP007 "Fix safety" paragraph reads "This rule's fix is marked as unsafe, as it may lead to runtime errors when alongside libraries that rely on runtime type annotations, like Pydantic, on Python versions prior to Python 3.10..." The filer of astral-sh#17062 hit the fix on Python 3.10+ (where the source explicitly marks the fix safe) and was confused about what the docs were warning about. In the thread, ntBre clarified the actual semantics ("the fix is marked unsafe on Python versions prior to Python 3.10 but safe on and after 3.10") and pjonsson suggested flipping the sentence around. I rewrote the paragraph to mirror the code: the fix is safe when target is 3.10+ and there are no comments, otherwise it's unsafe for one of two reasons (pre-3.10 runtime introspection like Pydantic can `TypeError` on `X | Y`, or comments inside the original would be dropped during rewrite). The unusual-inner-expression caveat is kept. I applied the same rewrite to UP045's docstring because it had the same wording. Docstring-only. `cargo dev generate-all` and `uvx prek run --from-ref main` both pass. fixes astral-sh#17062 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
anishgirianish
pushed a commit
to anishgirianish/ruff
that referenced
this pull request
May 28, 2026
…5288) The current UP007 "Fix safety" paragraph reads "This rule's fix is marked as unsafe, as it may lead to runtime errors when alongside libraries that rely on runtime type annotations, like Pydantic, on Python versions prior to Python 3.10..." The filer of astral-sh#17062 hit the fix on Python 3.10+ (where the source explicitly marks the fix safe) and was confused about what the docs were warning about. In the thread, ntBre clarified the actual semantics ("the fix is marked unsafe on Python versions prior to Python 3.10 but safe on and after 3.10") and pjonsson suggested flipping the sentence around. I rewrote the paragraph to mirror the code: the fix is safe when target is 3.10+ and there are no comments, otherwise it's unsafe for one of two reasons (pre-3.10 runtime introspection like Pydantic can `TypeError` on `X | Y`, or comments inside the original would be dropped during rewrite). The unusual-inner-expression caveat is kept. I applied the same rewrite to UP045's docstring because it had the same wording. Docstring-only. `cargo dev generate-all` and `uvx prek run --from-ref main` both pass. fixes astral-sh#17062 --------- Co-authored-by: Brent Westbrook <brentrwestbrook@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The current UP007 "Fix safety" paragraph reads "This rule's fix is marked as unsafe, as it may lead to runtime errors when alongside libraries that rely on runtime type annotations, like Pydantic, on Python versions prior to Python 3.10..." The filer of #17062 hit the fix on Python 3.10+ (where the source explicitly marks the fix safe) and was confused about what the docs were warning about.
In the thread, ntBre clarified the actual semantics ("the fix is marked unsafe on Python versions prior to Python 3.10 but safe on and after 3.10") and pjonsson suggested flipping the sentence around. I rewrote the paragraph to mirror the code: the fix is safe when target is 3.10+ and there are no comments, otherwise it's unsafe for one of two reasons (pre-3.10 runtime introspection like Pydantic can
TypeErroronX | Y, or comments inside the original would be dropped during rewrite). The unusual-inner-expression caveat is kept. I applied the same rewrite to UP045's docstring because it had the same wording.Docstring-only.
cargo dev generate-allanduvx prek run --from-ref mainboth pass.fixes #17062