Code linter and automatic formatter

[MarshallAsch]

Sometimes humans write bad looking code, we do not want bad looking code. Luckily for us some people have already written code that will take our code and make it look nice and ✨shiny✨, or yell at us when it is bad.

This will much improve the developers quality of life by reducing the number of PR comments that look like

• "fix your white space"
• "why do you have so many empty lines?????????"
• "name your things consistently, stop going back and forth between, cammelCase, snake_case, and BoGlEcAsE, ....."

Potential tools to use (I have not looked into any of these yet)

• PyType
• pylint
• flake8
• autopep8

---

Progress:

• [x] pick an automatic formatter
• [x] use the formatter for new modules, and update the style guide
• [x] set up automatic linting in CI
• [ ] convert old modules to the new code style

[jonasmalacofilho]

In the area of code formatting tools, I generally prefer the "format everything" approach, instead of the "just fix format 'issues'" ones.

I suggest something like:

• psf/black (see experiment)
• google/yapf

As far as I know, black is the current favorite in the Python community... (not that this matters a lot).

---

The other type of formatters, which are based on linters, don't change code that they see as already acceptable. But what they see as acceptable has little to do with looking good, or even being PEP 8 compliant, in all but the most trivial cases.

The "format everything" approach is more honest: yes, it may sacrifice some super elegant hand formatting, but it's consistent and saves everyone's time, not just the time of the more style-disinterested developers.

Also, code style is more than just formatting, so style-disinterested developers are probably still going to get some "hey, please do something sensible here" comments.

Finally, just for the record, I don't think neither of us has ever been as obnoxious as the examples you mentioned : )

[jonasmalacofilho]

@MarshallAsch,

I'm thinking of starting to adopt black on all new files, and leave the messy conversion of old files for later.

I've tried to convert the old files a few times already, but it always looks so messy: in some cases the code wasn't great to start off, and in others black differs too much from what we were using. Without some tweaks or, in some cases, moderate refactoring, the end result looks worse than what we started with...

That said, I'm missing black when doing new work. Also, this would allows to stop adding more files that would later need to be converted.

What do you think?

The configuration and changes to the style guide I'm considering are:

<!-- docs/developer/style-guide.md -->

[ ... ]

## General guidelines

This section has yet to be written, but for a start...

Read [PEP 8], then immediately watch Raymond Hettinger's [Beyond PEP 8] talk.
Write code somewhere between those lines.

In this repository, newer drivers are usually better examples than older
ones.  Experience with the domain helps to write better code.

Try to keep lines around 100-ish columns wide.

Be consistent within a given module; *try* to be consistent between similar
modules.

We are starting to adopt [psf/black], and new files should be formatted with
it.  They should also have the following top-level comment after the module
docstring:

```py
# uses the psf/black style

Old files should be maintained in their current style until a comprehensive conversion is performed; black has already been configured to ignore them.

Nits for old files

For new files, use the black code style, with 100-column lines.

For old files:

• indent with 4 spaces;
• prefer to continue lines vertically, aligned with the opening delimiter;
• prefer single quotes for string literals, unless double quotes will avoid some escaping;
• use f-strings whenever applicable, except for logging messages (more about those bellow) which wont necessarily be shown;
• use lowercase hexadecimal literals.

Grouping and sorting of import statements

[ ... ]

```toml
# pyproject.toml

[tool.black]
line-length = 100
target-version = ["py37"]
extend-exclude = '''
  /*
    Exclude old files, pending a future conversion.  By default, new files
    should *not* be excluded.

    To have all items start with |, this isn't a real comment; still, it
    shouldn't match any real path.
  */
  | (^/conftest\.py$)
  | (^/extra/contrib/fusion_rgb_cycle\.py$)
  | (^/extra/linux/generate-uaccess-udev-rules\.py$)
  | (^/extra/windows/LQiNFO\.py$)
  | (^/liquidctl/__init__\.py$)
  | (^/liquidctl/cli\.py$)
  | (^/liquidctl/driver/__init__\.py$)
    [ ... ]
  | (^/tests/test_smart_device\.py$)
  | (^/tests/test_smbus\.py$)
  | (^/tests/test_usb\.py$)
'''

(My staging may still have a slightly older version of these changes, since my PR #428 depends on it).

[MarshallAsch]

Also, this would allows to stop adding more files that would later need to be converted.

I agree with that, something is better than nothing.

I took a quick glance at black again, I looked at it more when this was originally discussed and I still think its good.

[jonasmalacofilho]

We have adopted black and there's now a style check action running in CI.

However, I don't think we'll get around to converting old modules to the new style all at once: it's a significant amount of work (t not make them worse in the process), and also the type of thing that has to happen when there is no other work in progress.

Since we also don't want to make git blame useless, the remaining alternative is to gradually update modules when, for different reasons, we're making large scale changes to them.

That said, if anyone volunteers to do the big one-time-conversion, accepting that they'll have to wait for the timing to be right, that would awesome!