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

Remove pyupgrade from our local dev tools and CI #11700

Closed
adiroiban opened this issue Oct 5, 2022 · 3 comments · Fixed by #11701
Closed

Remove pyupgrade from our local dev tools and CI #11700

adiroiban opened this issue Oct 5, 2022 · 3 comments · Fixed by #11701

Comments

@adiroiban
Copy link
Member

JP complains that it breaks the code.

Glyph

@adiroiban: I think pre-commit is great (perhaps I differ from jean-paul. here) but pyupgrade is just wrong
it makes changes that cause unit tests to fail
we should remove it from the pipeline, I'm not sure why it got added in the first place

@exarkun
Copy link
Member

exarkun commented Oct 5, 2022

Specifically:

  • pyupgrade is currently rewriting Optional[T] as T | None even though this is incompatible with Python 3.7 and we pass --py36-plus
  • pyupgrade is currently raising a bunch of ImportErrors on every run. They mostly look like:
Traceback (most recent call last):
  File "/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/bin/pyupgrade", line 5, in <module>
    from pyupgrade._main import main
  File "/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/lib/python3.9/site-packages/pyupgrade/_main.py", line 30, in <module>
    from pyupgrade._data import FUNCS
  File "/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/lib/python3.9/site-packages/pyupgrade/_data.py", line 126, in <module>
    _import_plugins()
  File "/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/lib/python3.9/site-packages/pyupgrade/_data.py", line 123, in _import_plugins
    __import__(name, fromlist=['_trash'])
  File "/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/lib/python3.9/site-packages/pyupgrade/_plugins/pep584.py", line 5, in <module>
    from tokenize_rt import List
ImportError: cannot import name 'List' from 'tokenize_rt' (/home/exarkun/.cache/pre-commit/repofhbuy0hk/py_env-default/lib/python3.9/site-packages/tokenize_rt.py)

@adiroiban
Copy link
Member Author

adiroiban commented Oct 5, 2022

The tokenize is due to older pyupgrade version.

The package metadata for older versions (and I think that event for current version) of pyupgrade instruct that any recent tokenize_rt package.

But a never version of tokenize_rt broke compatibility with older pyupgrade versions.

I think this is fixed in the latest version... but for now we can just remove pyupgrade from our tool chain.


It looks like both libraries have the same maintainer...so I guess backward compatibility is not a high priority for these libraries.

@glyph
Copy link
Member

glyph commented Oct 5, 2022

Thanks for the issue & PR @adiroiban, and thanks @exarkun for the very specific example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants