big pep8 formating

[un_pogaz]

I'm currently preparing a big pep8 formating commit (damn, Ruff is incredibly powerful).
It's far from be ready, I'm still figuring out how to do things properly, but in this phase of experimentation, a two questions have arisen about which files I should format and which not, and to what scope. While the main app 'calibre' will be the point of reference, it's much less clear for others sections like:

• the various build/setup/bypy sripts => need to be updated to Python 3 or should we keep Python 2 compatibility?
• the "embed third party" (like 'polyglot', 'templite', 'tinycss', 'css_selectors' and 'odf') should it be modified or kept as it, and which ones?

I also plan to add some rules to the existing ones, such as "D" docstyle, "Q" quotes and surely many others, BUT in general, I'm just beginning to experiment and explore which rules to use and which not.

And no, be assured that I will not use "ruff format". My aim goal is to standardize the code, all while preserving at maximum has possible the coding conventions you implicitly created.

PS: I already know that "metadata sources" and the "book store" code need to be keeped to Python 2 compatibility.

[kovidgoyal]

pep8 formatting is done already for the entire code base. I dont like all of pep8 so some rules are relaxed, see pyproject.toml for details. And these are enforced by running

./setup.py check

which is run as part of the release process by me and also periodically at other times, hence all the pep8 commits.

[un_pogaz]

Okay, I will keep that in mind.

I agree that not all pep8 rules are relevant and must be strictly applied, hence my caution, but there are various improvent and inconsistency-fix to do. A lot of little things.

Eventualy, we can apply more strictly the rules once by time, to keep a clear code, which is kind was became the project here. I'll see how I'd get organized in the end, I probaly will leave it a "strict-pep8.toml" file and add a option "--strict-pep8" to the './setup.py check' to future use, in years.

[un_pogaz]

Um, things are progressing but I'm faced with a dilemma:
At the moment, I've decided to follow your example of not modifying "Py2 legacy format strings", BUT this big project is also an opportunity to get rid of them once and for all and convert the to new format (with all that imply of translation string change).

Your choice?

PS: Tomorow, I will perform that at least for the not translation string.

[kovidgoyal]

I'd rather not create exra work for translators for something that has no actual effect. Also how are you changing the strings? Using a tool or by hand?

[un_pogaz]

Yeah, the nigth give me the same advice.

For this one (legacy format strings), I will use a option of Ruff that do it automaticly for the vast of them, do it manualy for the others, then reverse any translation string that will be affected before commit. I know that this one will be the more tricky, so I will be very carefull.
Double carefull in view of the mistake from the previous pull request, who could have been avoided if I had made this before to fix the translation strings .


Beside, here what my workflow and the result will take form:

• I perfom several fix and commit then by category, so that you can easly read the changes which are similar
  • Eventualy a commit with a single rules errors fix, if the change a more caution to make/read
• A rebase frequently my commits, so that the final pull request will be have none merge conflict

[kovidgoyal]

Sounds like a lot of work

[un_pogaz]

Finaly, I drop the project convert Py2 legacy format strings. To much change, to much risk of errors in one single commit.

I keep the idea on a the side and I will perfom this convertion little step by little step later, acrosse various version of Calibre.

[un_pogaz]

Oh, great thank you for moving the strict-pep8 to the default behavior, is far beyond I excepted for this project.

Since so, a information:
One of the advantages of having several config files was that you could have one safe for the auto-fix, and a other less safer for a manual checking. I realize this is irrelevant, but it has some consequences to keep in mind for a more regular use.

The first is that the 'preview' flag seems to change Ruff behavior in a way that is a little more profound than just a larger set of rules, leading to the global deactivation of the very helpfull 'F841 unused variable'. The issue is reported, but fix could be take a long time coming.
https://github.com/astral-sh/ruff/issues/8884

The other is the enlabing of 'RUF' rules. If the others groupes rules added during this project are well scoped, the 'RUF' groupe is not. This is the way out for some very good rules that not fit elsewhere, but also many others that we probably don't want. So, it's a group to keep an eye on when linting a new errors after upgrade Ruff.

[kovidgoyal]

Yes I am aware of the tuple unpacking unused var issue. As for RUF group introducing unwanted rules with upgrades, we will see over time how that goes. It's easy enough to disable a rule if it doesnt suit us.