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

Request: More autofixes for redundant-none-literal/PYI061 #14537

Closed
Avasam opened this issue Nov 22, 2024 · 8 comments
Closed

Request: More autofixes for redundant-none-literal/PYI061 #14537

Avasam opened this issue Nov 22, 2024 · 8 comments
Assignees
Labels
accepted Ready for implementation fixes Related to suggested fixes for violations help wanted Contributions especially welcome

Comments

@Avasam
Copy link
Contributor

Avasam commented Nov 22, 2024

def get_rotation(rotation: float | Literal[None, "horizontal", "vertical"]) -> float: ...

def spy(
    Z,
    precision: float | Literal["present"] = ...,
    marker=...,
    markersize=...,
    aspect: Literal["equal", "auto", None] | float = ...,
    origin: Literal["upper", "lower"] = ...,
    **kwargs,
) -> tuple[AxesImage, Line2D]: ...

class Axes(_AxesBase):
    def spy(
        self,
        Z,
        precision: float | Literal["present"] = 0,
        marker=...,
        markersize=...,
        aspect: Literal["equal", "auto", None] | float = "equal",
        origin: Literal["upper", "lower"] = ...,
        **kwargs,
    ) -> AxesImage | Line2D: ...
  • The command you invoked
    ruff check --fix --preview --select=PYI061 --isolated

  • The current Ruff version (ruff --version).
    ruff 0.8.0

@AlexWaygood AlexWaygood added fixes Related to suggested fixes for violations help wanted Contributions especially welcome accepted Ready for implementation labels Nov 22, 2024
@kiran-4444
Copy link
Contributor

I’d like to take this up.

@kiran-4444
Copy link
Contributor

So, if I understand this correctly the rule PYI061 is working for snippets like this:

from typing import Literal

a: Literal[None]
b: Literal[1, 2, 3, "foo", 5, None]

which gets auto-fixed to:

from typing import Literal

a: None
b: Literal[1, 2, 3, "foo", 5, None]

Here, I can see that the second statement isn't getting fixed. Is this the issue about? To fix things like this adhering to PYI061?

@Avasam
Copy link
Contributor Author

Avasam commented Dec 6, 2024

@kiran-4444 Exactly. Even the error message alludes to this being possible to autofix, just not implemented:

PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
  |
3 | a: None
4 | b: Literal[1, 2, 3, "foo", 5, None]
  |                               ^^^^ PYI061
  |
  = help: Replace with `Literal[...] | None`

@InSyncWithFoo
Copy link
Contributor

This seems to have been resolved by #14872.

@Avasam
Copy link
Contributor Author

Avasam commented Dec 15, 2024

Yep, Looks like this is closed by #14872 !

@Avasam Avasam closed this as completed Dec 15, 2024
@Avasam
Copy link
Contributor Author

Avasam commented Jan 28, 2025

Hmmm, I'm on 0.9.3 and all three of my example cases don't yet autofix:
ruff check --fix --unsafe-fixes --preview

stubs\matplotlib\axes\_axes.pyi:577:42: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
    |
575 |         marker=...,
576 |         markersize=...,
577 |         aspect: Literal["equal", "auto", None] | float = "equal",
    |                                          ^^^^ PYI061
578 |         origin: Literal["upper", "lower"] = ...,
579 |         **kwargs,
    |
    = help: Replace with `Literal[...] | None`

stubs\matplotlib\pyplot.pyi:715:38: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
    |
713 |     marker=...,
714 |     markersize=...,
715 |     aspect: Literal["equal", "auto", None] | float = ...,
    |                                      ^^^^ PYI061
716 |     origin: Literal["upper", "lower"] = ...,
717 |     **kwargs,
    |
    = help: Replace with `Literal[...] | None`

stubs\matplotlib\text.pyi:16:44: PYI061 `Literal[None, ...]` can be replaced with `Literal[...] | None`
   |
14 | from .transforms import Bbox, Transform
15 |
16 | def get_rotation(rotation: float | Literal[None, "horizontal", "vertical"]) -> float: ...
   |                                            ^^^^ PYI061
17 |
18 | class Text(Artist):
   |
   = help: Replace with `Literal[...] | None`

Found 3 errors.

@charliermarsh
Copy link
Member

Fixes just fine for me in the playground: https://play.ruff.rs/fd2b0091-14e2-44e2-9208-983d8e9c1862

@Avasam
Copy link
Contributor Author

Avasam commented Jan 28, 2025

I get it: on Python 3.9 the fix isn't offered at all rather than being "unsafe" (or using Union). I must've tested on 3.10 when I marked the issue as closed.
#14872 (review)

We can't safely offer the fix with the | syntax on Python <=3.9, as the | syntax for unions was added in Python 3.10. Possibly we could in the future use typing.Union for the fix and offer it even on older versions of Python, but I think we can leave that to another PR :-)

So I guess the fix being marked as unsafe but available was also left aside in that PR. I'll open a different issue for that specific use-case !

Done here: #15795

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation fixes Related to suggested fixes for violations help wanted Contributions especially welcome
Projects
None yet
Development

No branches or pull requests

5 participants