[pre-commit] Add pyupgrade back as a manual stage

[kloczek]

No description provided.

[Pierre-Sassoulas]

I'm wondering it we should keep pyupgrade in the pre-commit hooks then. ruff was at feature parity at some point but not anymore it seems. Or we could wait for ruff to keep up with feature parity and close this MR until then.

[The-Compiler]

This might just be a bug in ruff, with some special case (maybe the **locals()?) that made it miss this particular case?

[kloczek]

This might just be a bug in ruff, with some special case (maybe the **locals()?) that made it miss this particular case?

ruff still is not able to upgrade the code between python version so IMO it make sense to have both in pre-commit.

BTW soon in Oct this year python 3.8 will be EOSed.
Currently pyupgrade --py39-plus causes some pytest self test issues.
It wold be good if someone will try to review those generated changes to adapt pytest code and/or report some issues in pyupgrade.
Even more units are failing with pyupgrade --py3{9,10,11,12}-plus as well.
Just in case .. it is not critical but more ItWouldBeGoodToHave™️ ..

[The-Compiler]

ruff still is not able to upgrade the code between python version so IMO it make sense to have both in pre-commit.

ruff has integrated pyupgrade functionality for a long time already (possibly since the beginning?).

[kloczek]

ruff has integrated pyupgrade functionality for a long time already (possibly since the beginning?).

Just try to take few modules and filter over ruff and than over pyupgrade.
I understand that this was intention of the ruff. Reality is that pyupgrade is able to generate much more changes.

[The-Compiler]

You'll need to enable the respective rules (like pytest does).

[kloczek]

You'll need to enable the respective rules (like pytest does).

And I've tested that with those rules several times.
ruff still is ot able to filter the same way code as pyupgrade.
Really .. please try test that.

[Pierre-Sassoulas]

We activated all of ruff's pyupgrade messages:

pytest/pyproject.toml

Line 110 in 0070151

"UP", # pyupgrade

So yes, not feature parity, but the diff of this MR is the only remaining change pyupgrade have to apply in 4 months. (#11911 merge on 2024-02-02). Maybe there were a lot of diff we could have had faster and ruff caught up fast enough to not have a big diff now but I'm not sure it's worth the extra wait for each commit, for each developers, for 4 months.

Applying pyupgrade manually when each python interpreter dropped is also a possibility. That way we don't have to wait longer for each git commit and we can still benefit from pyupgrade's features not included in ruff, eventually.

[nicoddemus]

Applying pyupgrade manually when each python interpreter dropped is also a possibility. That way we don't have to wait longer for each git commit and we can still benefit from pyupgrade's features not included in ruff, eventually.

I'm OK with this, that's what I do in my other projects: I apply pyupgrade as part of the process of dropping a supported Python version. Of course, in pytest this would open the possibility of a "obsolete" pattern being re-introduced, but I think this is rare/not really a problem in general.

[RonnyPfannschmidt]

We could also just report findings upstream so that by the time we drop,ruff has parity

[Pierre-Sassoulas]

I pushed what I think is a good compromise: added pyupgrade back as a manual stage, this make it easily launchable with pre-commit run --hook-stage manual pyupgrade -a when we drop an interpreter, or when the pre-commit autoupdate upgrade the hooks (or anytime really) in 8f61d5d.

[nicoddemus]

TIL

[Pierre-Sassoulas]

Previously added pylint this way so can be launched similarly with pre-commit run --hook-stage manual pylint -a :)

[kloczek]

Thank you 👍