On Tue, Jun 24, 2025 at 3:34 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes: > > > This patch is fully automated, using pymagic, isort and autoflake. > > > > Create a script named pymagic.sh: > > > > ========================= > > > > pyupgrade --exit-zero-even-if-changed --keep-percent-format \ > > --py39-plus "$@" > > > > autoflake -i "$@" > > > > isort --settings-file python/setup.cfg \ > > -p compat -p qapidoc_legacy -p iotests -o qemu "$@" > > ========================= > > > > Then, from qemu.git root: > > > >> find . -type f -name '*.py' | xargs pymagic > >> git grep --name-only "#!/usr/bin/env python" | xargs pymagic > > > > This changes a lot of old Pythonisms, but in particular it upgrades the > > old Python type hint paradigm to the new 3.9+ paradigm wherein you no > > longer need to import List, Dict, Tuple, Set, etc from the Typing module > > and instead directly subscript the built-in types list, dict, tuple, > > set, etc. The old-style annotations are deprecated as of 3.9 and are > > eligible for removal starting in Python 3.14, though the exact date of > > their removal is not yet known. > > > > pyupgrade updates the imports and type hint paradigms (as well as > > updating other old 'isms, such as removing the unicode string > > prefix). autoflake in turn then removes any unused import statements, > > possibly left behind by pyupgrade. Lastly, isort fixes the import order > > and formatting to the standard we use in qemu.git/python and > > scripts/qapi in particular. > > > > Signed-off-by: John Snow <js...@redhat.com> > > [...] > > > 448 files changed, 1959 insertions(+), 1631 deletions(-) > > *Ächz* > Gesundheit. > > I hate it when people ask me to split up my mechanical patches... > > One split is by subsystem / maintainer. I've done this a few times, and > it's quite a bother. Questionable use of your time if you ask me. > I'd prefer not to unless it is requested of me specifically. I don't think most maintainers really care about the nuances of Python and as long as their stuff continues to work they're not going to mind much. Or, to be frank: I don't think this series would ever garner enough review and attention to warrant the labor it'd take to tailor it to such a review. It's mechanical, it's boring, it should be fine. I switched from a manual patch series to a tool-driven one specifically to make it more mindless and less interesting, and going through and splitting it back out is ... eh. I would prefer not to. > > There's another split here... Your pymagic.sh runs three tools. If you > commit after each one, the patch splits into three. > I use all three because each one alone isn't sufficient to then pass the static analysis checks, they each do a little bit of damage that another tool corrects afterwards. pyupgrade works to modernize syntax, but leaves impotent import statements hanging. autoflake removes those impotent imports. isort fixes the import statement ordering and formatting to our standard. (And then I do some manual fixups to fix the linting tests where things were auto-formatted suboptimally.) I can still split it out for review purposes, like I did here with some manual fixups appended to the end. Just, for merge, they'll be combined by necessity as a result of our no-regressions-for-bisect rule. > > I understand you pass --py39-plus to pyupgrade to get the type hints > modernized. If you run it without --py39-plus for all the miscellaneous > upgrades, commit, then run it with --py39-plus for just the type hint > upgrades, commit, the last patch splits again. > I can try it! I actually didn't try running it without py39-plus at all, so I don't know what that'll do. but no harm in an experiment. > > Thoughts? First and foremost I just thought it'd be good to get this mechanical change squared away in one giant patch so we could add this one singular horrible mega-commit into the git blame "ignored commits" list to minimize the impact of the "flag day". This upgrade will have to happen "eventually" but it needn't be "right now", but I figured it'd be good to get it out of the way... or put another way, "better my mess than someone else's". --js
