Add linters configuration, reformat whole code

[antoni-szych-rtbhouse]

Hi,

As black and isort became the standard tools to enforce uniform style within the whole codebase, I've came to a conclusion that these may be helpful also here, as the style currently varies in different parts of the codebase.

I've added:

• black
• isort
• moved the pytest config to pyproject.toml

I've reformated the code according to black and isort configuration.

Topics that are very open for discussion:

• line lengths. Black gives default of 88 characters per line. ~Given that nearly all displays currentlyt have 16:9 / 16:10 aspect ratio instead of 4:3, 120 characters per line also seems reasonable (which I've set in the configuration in pyproject.toml)~ Default of 88 chars is used. However this setting is entirely dependent on the personal preference and can be easily changed to any other value. more reading
• string normalization. Black by default converts all strings that do not contain double quotes " to double-quoted ones instead of singe-quoted '. However, some people have strong opinion regarding quoting style and allow double-quotes only in docstrings. ~I've left the default normalization on, but performed the normalization in separate commit. If needed, that commit can be easily skipped and the configuration can be changed to include skip-string-normalization~. more reading String normalization was dropped from this PR.

[ds-cbo]

"Given that nearly all displays currentlyt have 16:9 / 16:10 aspect ratio instead of 4:3, 120 characters per line also seems reasonable"

Counterpoint: I often have 2 terminals side-to-side on my 16:9 screen, and then it only fits up to 100 characters comfortably. It also depends on font-size, which is still very personal nowadays. I would keep the default value of black (80 + 10% wrap margin)

[ds-cbo]

"I've left the default normalization on"

I don't have strong opinions on quotes, but I do have a strong opinion on git history: Please try to keep the changes minimal when introducing a new tool. Other developers will git annotate a file and 50% of the lines will get "black 2023/12/14" as annotation, which is completely useless.

[alecthomas]

Counterpoint: I often have 2 terminals side-to-side on my 16:9 screen, and then it only fits up to 100 characters comfortably. It also depends on font-size, which is still very personal nowadays. I would keep the default value of black (80 + 10% wrap margin)

Counter-counterpoint: I suspect the majority of people, myself included, prefer 100 or 120 columns, and for the minority who don't, having to scroll horizontally every now and then is not the end of the world. I have to do it myself sometimes on smaller screens, and I'm fine with it.

Edit: additionally, 100+ columns is what Voluptuous is currently using.

[antoni-szych-rtbhouse]

"I've left the default normalization on"

I don't have strong opinions on quotes, but I do have a strong opinion on git history: Please try to keep the changes minimal when introducing a new tool. Other developers will git annotate a file and 50% of the lines will get "black 2023/12/14" as annotation, which is completely useless.

This can be actually easily fixed both locally and for github blame: we just need to put the commit sha into the .git-blame-ignore-revs file. Note that it will have to be added after merging this PR, as we don't know the merge commit sha in advance :)

Note that the same logic applies to any refactoring effort like using f-strings or getting rid of object in class Foo(object) (redundant in python3). Even if .git-blame-ignore-revs wouldn't exist (which it does since git 2.23 from 2019), I don't think that more complicated browsing file history should really prevent anyone from code refactoring. Especially if the "complicated way" involves clicking 1 button more:

github:

pycharm:

vscode + git lens (the most popular git extension for vscode):

[antoni-szych-rtbhouse]

Hi @ds-cbo @alecthomas can you please take a look at the updated version? :)

[ds-cbo]

I have no new complaints, but I'm also not responsible for this project

[antoni-szych-rtbhouse]

@alecthomas could you please take a look? :) I think all the major obstacles were resolved with the change in line length and few fmt:off/ons and ignores.

[alecthomas]

Mmm yeah, one last change please: can you make the default string quoting ' instead of ". That is the default in this project, and leaving it as-is will reduce the diff size considerably.

[antoni-szych-rtbhouse]

Mmm yeah, one last change please: can you make the default string quoting ' instead of ". That is the default in this project, and leaving it as-is will reduce the diff size considerably.

Sure, I've removed the string normalization commit (it was the last one) and the PR is smaller (+755 −433 vs +1,047 −730 previously).

I've also made a test and replaced all reasonable double quotes to single quotes in 231 lines. This would cause the diff to be +966 −648. Please let me know if case you would like me to commit this.

[ds-cbo]

This can be actually easily fixed both locally and for github blame: we just need to put the commit sha into the .git-blame-ignore-revs file. Note that it will have to be added after merging this PR, as we don't know the merge commit sha in advance :)

Now please don't forget to do that as well, now we have this:

[antoni-szych-rtbhouse]

Now please don't forget to do that as well, now we have this:

Done in #505, it will look like following (screenshot from my fork with this commit already merged):

As you can see, there are some lines annotated with the refactor commit, however these are only the new lines, which did not exist at all previously. However the important lines like 26-31, 36-40, 45-46, 51-53 show the pre-refactor annotation.