Don't upgrade generics - part 2

[Anthchirp]

I observe the same issue as #22, using v1.0.1.

Starting with the file

from __future__ import annotations

from typing import Any, Callable, Mapping

MessageCallback = Callable[[Mapping[str, Any], Any], None]

def func(x: Mapping[str, Any], y: Any) -> None:
    return None

callback: MessageCallback
callback = func

This originally runs without errors, but not after running pep585-upgrade has run:

$ python -V
Python 3.8.6
$ python test.py
$ pre-commit run
Upgrade type hints.......................................................Failed
- hook id: upgrade-type-hints
- exit code: 1
- files were modified by this hook

Fixing src/workflows/test.py

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

$ git diff
index 72539fc..56bc282 100644
--- a/test.py
+++ b/test.py
@@ -1,6 +1,7 @@
 from __future__ import annotations

-from typing import Any, Callable, Mapping
+from collections.abc import Mapping
+from typing import Any, Callable

 MessageCallback = Callable[[Mapping[str, Any], Any], None]

$ python test.py
Traceback (most recent call last):
  File "test.py", line 6, in <module>
    MessageCallback = Callable[[Mapping[str, Any], Any], None]
TypeError: 'ABCMeta' object is not subscriptable

[sondrelg]

Yeah this seems like an unfortunate bug if wanting to use this as a regular pre-commit hook. Would you be interested in contributing with a fix @Anthchirp?

[Anthchirp]

I think the solution is to basically not replace any assignments unless the minimum supported version is 3.9:

This is why starting with Python 3.9, the following collections become generic using __class_getitem__() to parameterize contained types

https://www.python.org/dev/peps/pep-0585/#implementation

So this would at least need another flag to set the minimum supported Python version. Would you be happy with that?

[Anthchirp]

The easier alternative is that you could just say the pre-commit hook is only supported for 3.9+ :slightly_smiling_face:

I just saw that this hook is currently 3.8+ only, however my projects still need to support 3.7, so I won't be looking further into this for now.

[sterliakov]

Same here:( I was really upset when encountered this (on project supporting exactly python 3.8+). Should I submit a PR with README change (to state that it is compatible only with 3.9+, not 3.8)?

[sondrelg]

The hook was originally written to upgrade a few codebases, and there was no other alternative at the time. Pyupgrade now does close to the same thing, so perhaps it's time to deprecate this instead 🤔

[sterliakov]

Pyupgrade is great, but lacks one feature I really wanted to have for consistency in large package: annotations future should be imported in all files (after a couple failing CI runs on 3.8, when I was forgetting to add it to previously empty __init__.py, for example). Also adding this line manually can result in huge diff (thanks to Pyupgrade), because it does the opposite: when this future is imported, some changes may apply. So it's kind of annoying to always add it manually and control that all files contain this future-import, and this hook is able to add wanted line.

[sterliakov]

Seems like I'm going to write my own hook for that: pyupgrade maintainer recommends using reorder-python-imports, but I prefer isort for a few reasons (I don't like one-per-line rule, although it reduces merge conflicts - for project with a few contributors it is not crucial) - so won't switch.

[sondrelg]

Pyupgrade is great, but lacks one feature I really wanted to have for consistency in large package: annotations future should be imported in all files

isort has a feature where it adds futures import to every file for you. I'll see if I can dig up an example.

[sondrelg]

You can do

[tool.isort]
profile = "black"
add_imports = "from __future__ import annotations"

[sterliakov]

Wow, thank you! I wasn't aware about this feature.

[sondrelg]

I'm officially deprecating this project, as pyupgrade now fills the gap this once did. The package can still be used, but I'm not intending to spend any more time maintaining it, so closing this for now 🙂