Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python formatting style #2690

Open
echoix opened this issue Feb 5, 2025 · 13 comments
Open

Python formatting style #2690

echoix opened this issue Feb 5, 2025 · 13 comments

Comments

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

I'd like to know your opinions on what automated formatting tool you'd be open to use.

I have two options to suggest. There's the classic Black, a reference in the domain. It's stable style changes once per year, at their first release of the year (2025 style is out).

Second option is Ruff's formatter (ruff format). It is a drop in replacement for black, except for small known differences where they decided that black's style would be worse than theirs. It is waaaaay faster (fractions of a second for a 350 000 line project) and the developer experience is better. It can also format examples in docstrings, whether they are in markdown or rst style.

Having a constant, tool enforced Python style reduces the time spent on debating on useless style questions, as it will always be the same for every contributor. Then, when making changes to modernise an usage pattern for example, will have a reduced diff to review, as there will be no unrelated changes since the formatting is always enforced.

@swt2c
Copy link
Collaborator

swt2c commented Feb 6, 2025

Honestly, I'm a little uneasy about adopting a code formatter, mainly about reformatting existing code:

  • It would really mess up the utility of git blame which I rely on a lot
  • It could introduce bugs and we are far from 100% test coverage

@echoix
Copy link
Contributor Author

echoix commented Feb 6, 2025

Honestly, I'm a little uneasy about adopting a code formatter, mainly about reformatting existing code:

  • It would really mess up the utility of git blame which I rely on a lot

A one-time change, yes, but it's a change by itself too.
Initially, a couple years ago, members of the GRASS GIS project (a 40+ year old project that lived through multiple transitions) thought the same, and heavily used the .git-blame-ignore-revs for that case. But in hindsight, hiding commits with this ends up more difficult to follow history. They never regretted having code formatted long time ago, even when tooling was not as mature as they are now. There have been less changes to dig out in the years after that, as the style was consistent.

Fun fact: I regularly see some less changed files of the GUI side that still show some large parts of the code still the same since they were ported to wxPython from a C++ wxWidgets implementation, years and years ago

  • It could introduce bugs and we are far from 100% test coverage

This is the first time I hear about this. Talking about formatting only, there's no real situations where it could be introduced, except for things like parenthesised multiline with statements, that work with Python 3.10. These are things where more syntax constructs are allowed with newer Python versions. That's why setting the requires-python field in pyproject.toml (for ruff), or a config setting in black (a list of Python versions the code should support), stored in the pyproject.toml file too, makes these tools adapt to what is allowed to be used.

@echoix
Copy link
Contributor Author

echoix commented Feb 6, 2025

If we manage to prove, a file at a time, for every supported Python versions, that the AST is the same before and after, is it strong enough?

Something like described in this blog
https://idle.nprescott.com/2017/formatting-python.html

@Metallicow

This comment has been minimized.

@echoix
Copy link
Contributor Author

echoix commented Feb 6, 2025

if : of if T:

That is the shortest 'if True run:' statement I've ever seen. Run a formatting program on it....

You might have been as comical as 'Guido Van Rossum' (which, by the way, wasn't being too serious, HAHA).
I have a list of optimizations(try it), which might be more readable than others. No one ever checks this shit? WHY?

Author of funnyshit.ast

I'm not sure I fully understand your point is here... Is it trolling? What is this supposed to suggest in the context of finding out what the Python style should be here?

Anyways, your link to the repo is quite nice for you to have tried out. These kinds of things change per Python version, and 3.11 changed a lot performance-wise.

If we admit that performance was the topic of this discussion (it was yet), I agree that it is possible to refactor Python to a point where it would be more performant, in that specific case, for that specific Python version (and sometimes even per platform). But would it be more readable?
It's Python guys... we are not exactly here for the top performance, for the efficiency performance when writing, reading and using code, otherwise we're missing the boat. Developer time, in open source, is kind of a scarce resource.

@Metallicow

This comment has been minimized.

@echoix
Copy link
Contributor Author

echoix commented Feb 6, 2025

I think some of the comments here should be marked hidden as off-topic by someone here with triage permissions or more.

I think I conclude that the community here (except for Scott), is not open to contributions and would rather keep no changes, so I'll go back to my other projects in the close future, hoping that wxPython improves by itself in the directions I wish when using it in my projects.

@Metallicow
Copy link
Contributor

Metallicow commented Feb 6, 2025

I think some of the comments here should be marked hidden as off-topic by someone here with triage permissions or more.

I think I conclude that the community here (except for Scott), is not open to contributions and would rather keep no changes, so I'll go back to my other projects in the close future, hoping that wxPython improves by itself in the directions I wish when using it in my projects.

I apologize @echoix I'm not sure how I offended you besides the only way to do so is running MY program.
DID YOU? Post your results...

We all at the wxPython Organization would like to SEE them...

test01(text):
    self.print(len(text))
test02(text)
    self.print(len(text))

@swt2c
Copy link
Collaborator

swt2c commented Feb 6, 2025

Honestly, I'm a little uneasy about adopting a code formatter, mainly about reformatting existing code:

  • It would really mess up the utility of git blame which I rely on a lot

A one-time change, yes, but it's a change by itself too. Initially, a couple years ago, members of the GRASS GIS project (a 40+ year old project that lived through multiple transitions) thought the same, and heavily used the .git-blame-ignore-revs for that case. But in hindsight, hiding commits with this ends up more difficult to follow history. They never regretted having code formatted long time ago, even when tooling was not as mature as they are now. There have been less changes to dig out in the years after that, as the style was consistent.

OK, yeah I have heard about .git-blame-ignore-revs. You don't recommend using it though?

Fun fact: I regularly see some less changed files of the GUI side that still show some large parts of the code still the same since they were ported to wxPython from a C++ wxWidgets implementation, years and years ago

  • It could introduce bugs and we are far from 100% test coverage

This is the first time I hear about this. Talking about formatting only, there's no real situations where it could be introduced, except for things like parenthesised multiline with statements, that work with Python 3.10. These are things where more syntax constructs are allowed with newer Python versions. That's why setting the requires-python field in pyproject.toml (for ruff), or a config setting in black (a list of Python versions the code should support), stored in the pyproject.toml file too, makes these tools adapt to what is allowed to be used.

I guess I was thinking of theoretical things like it messing up strange quoting situations. I've also seen things like isort create problems where there are import ordering constraints, but that probably isn't an issue for wxPython.

What would you recommend, black or ruff? I only have used black before.

@Metallicow
Copy link
Contributor

@swt2c I respect your decision on the matter.

I hate formatting, that's why I came up with a process to eliminate the convolution of dissenters.

Kind of nom-nomical huh...?

@DietmarSchwertberger
Copy link
Contributor

I don't see the added value.
In my own project the (manual) formatting is always such that it's most readable and such that code is not stretched into more lines/screens than necessary. I do not care about rules. A human being can decide better about readability than an algorithm.

When I contribute to projects, I try follow their style. If the style allows variation, then I will go for the one that's more readable.

@infinity77
Copy link
Contributor

I’m not that much of a contributor anymore, but for what is worth I am with @DietmarSchwertberger here. I am not super familiar with either black or ruff, but I have yet to see an instance of reasonably well written code become much more appealing after the application of a formatter.

Of course trying it on the codebase costs zero - but I would need to see a masterpiece before I get convinced.

@newville
Copy link
Contributor

newville commented Feb 7, 2025

similar opinioin to @infinity77 @DietmarSchwertberger. I am not much of a developer here, and I understand that mine is a minority opinion for many Python projects. But, I would strongly caution against using automated formatters such as Black. Python is readable - that's one of the main points of the language. Saying that an inconsistent number of whitespaces between operators or different uses on single and double quotes is always a problem and will cause "cognitive dissonance" in the reader is just absurd.

It is definitely OK to add linting and formatting style checks on new or changing code, with the goal of "Let's try to be consistent and only break consistency intentionally".

But, wholesale changes of existing code? Um, why? What problem does that solve?

You know, the thing is some person wrote the code that way. That code works. That code has been in the codebase for some time - maybe weeks, maybe decades. The existing text of that code is literally copyrighted.

So, why does someone think that applying a linter will add value? Is the motivation "let me deliberately change all of your code to meet my expectations"? FWIW, Black is even worse than that, as its formatting choices are simply terrible.

When someone comes to one of my projects and says "you should use Black or ruff" my answer is, and always will be, a firm and clear No. Writing code is a creative process, still mostly done by humans who intentionally write code. That process does not map well to "you must always do that in this particular way". Yes, be consistent and write good code for others to read. Saying that 99.5% consistency is not good enough is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants