arcade
arcade copied to clipboard
pythonarcade
Autoformatting
How do people feel about implementing (and enforcing) autoformatting in Arcade? Specifically:
- Black for code (120 char line length)
- Ruff for imports
Some considerations:
- we would likely need to disable this for sphinx purposes on some files
- git blame can be preserved (inconveniently) using this: https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame
As far as ruff goes, we already use it for linting, so using it for the import sorting is trivial.
Really just need to exclude the examples code from autoformatting as far as exclusions go, maybe the experimental directory because there are technically examples in that? Not sure autoformatting matters for any of those though.
I don't think we care about git blame preservation
Things that probably need to happen for this:
- A single, giant PR for initial conversion
- Added to the CI to check for formatting(probably not having CI actually do any formatting though)
- Add a tool to make.py to run it(CI could just use this probably)
- Update contributing docs for it
Isn't step one actually some kind of broader consensus? Who needs to sign off?
Autoformatting the imports may break a bunch of stuff at first, due to.. load bearing import order
Those steps were just like, assuming it was happening, we probably have very little load bearing import order left. The 3.0 work cleaned up a lot of Arcade import. I would probably be in favor of excluding __init__.py files from the formatting though, the top level one is probably a minefield
gui is already formated with ruff and I use pre-commit hooks to enforce it on my side.
TL;DR: I am very wary of this idea.
The most important part of this (imo) is knowing how to turn it off. The following probably need to be excluded:
make.py- examples (as Clepto said above)
arcade.experimental- Sphinx stuff
- The top-level
utilsfolder can be fragile (includes more Sphinx stuff)
We're refactoring the utils folder one step at a time (#2109), but it's probably best to leave it alone for now.
As to other auto-formatting options not mentioned above, there are good reasons to disable them in many cases. PEP-8 itself points out that following it dogmatically is bad. I don't think you're proposing we do so, but I'm bringing it up now so we can agree not to do so in the future.
#2141 just covered implementing black. Intended to follow up with import sorting separately, I’ll just reopen this for that.
Can we also do it with ruff? black and ruff are compatible, but ruff still wants to change >100 files, because it wants to do little more then black.
We could do it with ruff, I believe the reason it touches so much more is that Ruff hits docstrings slightly. At the least we can make a PR with it and see what the difference looks like.
The commit fails, because not all doc strings are shortened to a length of 100.
Do we want to keep it 100? Maybe I just go through the doc strings and format it.
Both ruff and black is not using 100 line limit : https://github.com/pythonarcade/arcade/pull/2260