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.

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

Revisit adding all PEP585 names to UP006 #15251

Open
MichaReiser opened this issue Jan 4, 2025 · 0 comments
Open

Revisit adding all PEP585 names to UP006 #15251

MichaReiser opened this issue Jan 4, 2025 · 0 comments
Labels
rule Implementing or modifying a lint rule

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Jan 4, 2025

#5454 added all PEP-585 names to UP006 but this caused multiple issues which is why I decided to revert it in #15250

We should revisit if we want this change and how we resolve possible conflicts, as summarized by a contributor:

I have raised a few issues with this change:

  • Duplication and inconsistency between pep585 annotation rules: UP006, UP035 #15246

    Even if the detection+fix ability is better now that these are implemented in UP006, something needs to be done about the fact that the newly added warnings are all duplicates of warnings and fixes that already exist in UP035.

    If this implementation becomes a little more polished, it probably should completely replace the implementation that's currently in UP035, there's really no reason to still keep that inferior implementation other than its battle-tested status. Although maybe for historic consistency, even if the implementation is unified towards UP006, the set of items that it triggers for should remain split between UP006 (covering only builtins such as list) and UP035 (covering others) like it's always been.

  • Strange fix behavior for UP006/NonPEP585Annotation fixer since ruff 0.8.5 #15245

    The fixer has bugs where it sometimes fails to kick in, and in corner cases kicks in with strange results.

For now I'd like to see this pull request reverted and relanded later after a more detailed discussion, because it has ongoing bugs and possibly introduces undesirable/unexpected changes to what the rules are supposed to cover.

Originally posted by @oprypin in #5454 (comment)

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

1 participant