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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

UP035 vs. flake8-pep585 #3388

Closed
ericbn opened this issue Mar 7, 2023 · 13 comments 路 Fixed by #3448
Closed

UP035 vs. flake8-pep585 #3388

ericbn opened this issue Mar 7, 2023 · 13 comments 路 Fixed by #3448
Assignees
Labels
bug Something isn't working

Comments

@ericbn
Copy link

ericbn commented Mar 7, 2023

Hi! 馃憢

First thank you for such amazing project, caching up so fast with almost all that has been done for Python linting so far.

I use flake8 with flake8-pep585 and noticed that Ruff's UP035 implementation yields incomplete results comparing to that. I'm using py39 as the target version in both flake8 and Ruff. I didn't compare it with pyupgrade, which I know is the reference implementation.

test.py

Minimal code snipped that reproduces the bug:

from typing import (
    AbstractSet,
    AsyncContextManager,
    AsyncGenerator,
    AsyncIterable,
    AsyncIterator,
    Awaitable,
    ByteString,
    Callable,
    ChainMap,
    Collection,
    Container,
    ContextManager,
    Coroutine,
    Counter,
    DefaultDict,
    Deque,
    Dict,
    FrozenSet,
    Generator,
    ItemsView,
    Iterable,
    Iterator,
    KeysView,
    List,
    Mapping,
    MappingView,
    Match,
    MutableMapping,
    MutableSequence,
    MutableSet,
    OrderedDict,
    Pattern,
    Reversible,
    Sequence,
    Set,
    Tuple,
    Type,
    ValuesView,
)

Ruff output

The command I've invoked: ruff --isolated --select UP --target-version py39 test.py

test.py:1:1: UP035 [*] Import from `collections.abc` instead: `AsyncGenerator`, `AsyncIterable`, `AsyncIterator`, `Awaitable`, `ByteString`, `ChainMap`, `Collection`, `Container`, `Coroutine`, `Counter`, `Generator`, `ItemsView`, `Iterable`, `Iterator`, `KeysView`, `Mapping`, `MappingView`, `MutableMapping`, `MutableSequence`, `MutableSet`, `Reversible`, `Sequence`, `ValuesView`
test.py:1:1: UP035 [*] Import from `re` instead: `Match`, `Pattern`
Found 2 errors.
[*] 2 potentially fixable with the --fix option.

ruff --version

ruff 0.0.254

flake8-pep585 output

Besides failing with the same as Ruff's UP035 above, flake8-pep585 also reports:

test.py:1:1: PEA001 typing.AbstractSet is deprecated, use collections.abc.Set instead. See PEP 585 for details
test.py:1:1: PEA001 typing.AsyncContextManager is deprecated, use contextlib.AbstractAsyncContextManager instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Callable is deprecated, use collections.abc.Callable instead. See PEP 585 for details
test.py:1:1: PEA001 typing.ContextManager is deprecated, use contextlib.AbstractContextManager instead. See PEP 585 for details
test.py:1:1: PEA001 typing.DefaultDict is deprecated, use collections.defaultdict instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Deque is deprecated, use collections.deque instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Dict is deprecated, use dict instead. See PEP 585 for details
test.py:1:1: PEA001 typing.FrozenSet is deprecated, use frozenset instead. See PEP 585 for details
test.py:1:1: PEA001 typing.List is deprecated, use list instead. See PEP 585 for details
test.py:1:1: PEA001 typing.OrderedDict is deprecated, use collections.OrderedDict instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Set is deprecated, use set instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Tuple is deprecated, use tuple instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Type is deprecated, use type instead. See PEP 585 for details

These are all mentioned under https://peps.python.org/pep-0585/#implementation and here's the flake8-pep585 implementation: https://github.com/decorator-factory/flake8-pep585/blob/master/flake8_pep585/rules.py

@charliermarsh
Copy link
Member

Yeah we seem to be missing some of these. Let me audit the list...

@charliermarsh charliermarsh added the bug Something isn't working label Mar 7, 2023
@charliermarsh
Copy link
Member

charliermarsh commented Mar 7, 2023

Okay so...

test.py:1:1: PEA001 typing.OrderedDict is deprecated, use collections.OrderedDict instead. See PEP 585 for details

This we're missing, I'm adding them (same with ChainMap and Counter from collections in Python 3.9+).

# test.py:1:1: PEA001 typing.AbstractSet is deprecated, use collections.abc.Set instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.AsyncContextManager is deprecated, use contextlib.AbstractAsyncContextManager instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.ContextManager is deprecated, use contextlib.AbstractContextManager instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Deque is deprecated, use collections.deque instead. See PEP 585 for details

We're missing these, I think because they're not "straight" replacements (you have to change the usages too, e.g., from AbstractSet to Set). I'll add warnings for them, but won't be able to fix them right now just given limitations in the fix API. (It'd require both changing the import and all the callsites.)

# test.py:1:1: PEA001 typing.Callable is deprecated, use collections.abc.Callable instead. See PEP 585 for details

I think we actually do catch this one, at least in my testing.

# test.py:1:1: PEA001 typing.Dict is deprecated, use dict instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.FrozenSet is deprecated, use frozenset instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.List is deprecated, use list instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.OrderedDict is deprecated, use collections.OrderedDict instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.Set is deprecated, use set instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.Tuple is deprecated, use tuple instead. See PEP 585 for details
# test.py:1:1: PEA001 typing.Type is deprecated, use type instead. See PEP 585 for details

These are covered by a different rule (UP006), which handles rewrites at the usage site (e.g., it changes List[int] to list[int]). Typically, UP006 fixes those usages, and then those are marked as "unused imports". But we could consider flagging them here too...

@ericbn
Copy link
Author

ericbn commented Mar 8, 2023

Thanks for the quick heads up!

It makes sense that the ones with different names will be harder to fix. Also makes perfect sense that the List vs. list could be fixed the way it currently works. Having them also as "UP" warnings maybe would be a nice addition just for sake of completeness.

# test.py:1:1: PEA001 typing.Callable is deprecated, use collections.abc.Callable instead. See PEP 585 for details

I think we actually do catch this one, at least in my testing.

I saw it was listed in Ruff's code, but it does not seem to work if it's imported along others:

$ echo 'from typing import Callable' >| test.py
$ ruff --isolated --select UP --target-version py39 test.py
$ echo 'from typing import Callable, Container' >| test.py
$ ruff --isolated --select UP --target-version py39 test.py
test.py:1:1: UP035 [*] Import from `collections.abc` instead: `Container`
Found 1 error.
[*] 1 potentially fixable with the --fix option.

@charliermarsh
Copy link
Member

Oh interesting, lemme look into that...

@charliermarsh
Copy link
Member

Ruff has that noted as a Python 3.10+ thing.

@charliermarsh
Copy link
Member

Ahh right, it's because that change was made in Python 3.9.2 (https://stackoverflow.com/questions/65858528/is-collections-abc-callable-bugged-in-python-3-9-1), so collections.abc.Callable actually isn't available on all Python 3.9 versions. This has come up before.

@charliermarsh
Copy link
Member

(Will add those remaining deprecation rules tomorrow, I have a PR in progress.)

@charliermarsh charliermarsh self-assigned this Mar 8, 2023
@ngnpope
Copy link
Contributor

ngnpope commented Mar 8, 2023

Ahh right, it's because that change was made in Python 3.9.2

See #2690

@charliermarsh
Copy link
Member

I'm a few days delayed on this (clearly) but still plan on adding these.

@ericbn
Copy link
Author

ericbn commented Mar 14, 2023

Hi. Thanks a lot for working on this.

These are covered by a different rule (UP006), which handles rewrites at the usage site (e.g., it changes List[int] to list[int]). Typically, UP006 fixes those usages, and then those are marked as "unused imports". But we could consider flagging them here too...

I see you even worked on these to be part of UP035! 鈽濓笍 馃帀

I double checked the test.py file with ruff 0.0.255 and I found a couple still not reported compared to flake8-pep585:

test.py:1:1: PEA001 typing.ContextManager is deprecated, use contextlib.AbstractContextManager instead. See PEP 585 for details
test.py:1:1: PEA001 typing.Type is deprecated, use type instead. See PEP 585 for details

As far as I checked, these are present on Python 3.9.0.

@charliermarsh
Copy link
Member

Oof, let me take one more look, thank you.

@charliermarsh
Copy link
Member

This time I actually looked through the list at https://peps.python.org/pep-0585/#implementation.

@ericbn
Copy link
Author

ericbn commented Mar 15, 2023

Cool! Thank you for the patience over this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants