search

tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.76k stars 5.51k forks source link

really drop python<=3.7 #3391

Closed kloczek closed 5 months ago

kloczek commented 5 months ago

Filter all code over pyupgrade --py38-plus.

bdarnell commented 5 months ago

Most of these actually date back to Python 3.0 or earlier (e.g. the default (object) base class was in 3.0, set literals in 2.7). I'm wary of change for the sake of change but it's probably time to change these and the tool helps make it safer. I've never liked the .format() method (introduced in 2.6), though - I prefer the % operator or f-strings. Looks like pyupgrade has an option to leave string formatting alone. (I wish it would let you run one rule at a time for ease of reviewing - get all of the (object) changes out of the way in one commit, etc)

I'm not sure about that second ruff commit - I think most if not all of those imports are needed for type checking. We already have flake8 that warns about unused imports so there shouldn't really be any.

kloczek commented 5 months ago

flake8 is more and more behind of what ruff is able to detect. Nevertheless with both commits test suite is OK (tornado.test.runtests) and pytest is failing the same way as without that those changes (as I've reported in https://github.com/tornadoweb/tornado/issues/3390)

bdarnell commented 5 months ago

flake8 is more and more behind of what ruff is able to detect.

I'm not in the market for a new linter (well, I may switch from flake8 to plain pyflakes now that black and pep8 are diverging). I care more about a high signal-to-noise ratio than just detecting more stuff (this is why I use pyflakes and not pylint).

Nevertheless with both commits test suite is OK (tornado.test.runtests)

It also needs to pass the linters (tox -e lint. Also tox -e docs although i don't think that's going to be an issue here). The current failure just needs black to be run after pyupgrade, and then we'll see if mypy is happy with the ruff changes.

kloczek commented 5 months ago

OK I've added commit with all changes generated by black.

bdarnell commented 5 months ago

I've got almost all the pyupgrade changes in a new PR, #3403. I had to hack on pyupgrade a bit to disable one change and then break things out into separate commits for easier review. I also found that running pyupgrade in two passes will convert percent formatting to str.format and then str.format to fstrings, which is nice.

kloczek commented 5 months ago

Thank you 👍