chore: add black as code formatter and Github Action

[denismaciel]

This PR does three things:

1. As discussed in #52, it adds Black as a code formatter.

   Black doesn't allow much customization, which I think is a good thing. The only option I changed from the default is to tell Black not to change single-quote ' to double-quote ". This is what the --skip-string-normalization does.

   To enforce single quotes wherever possible, I have added an extra pre-commit hook (double-quote-string-fixer).

   Preferring single-quotes over double ones is just me. Let me know if prefer the other way around and I will change it.

2. The PR adds pre-commit to the development tool set. I really like pre-commit. It makes it easy to lint the code on CI and enforce the coding standards.

3. The PR adds a Github Action to lint the code and run the tests on different platforms.

For a sample run of the CI workflow, see here.

I realize the PR might be a bit too opinionated on the choices, so do let me know if would like to have something different and I will gladly adapt.

Cheers!

[lervag]

Thanks! I've been very busy the last days, but I will be sure to look at this as fast as I possibly can!

[lervag]

1. As discussed in improvement ideas #52, it adds Black as a code formatter.

Great! I notice that the configuration for Black is "implicit" in the sense that it is provided as options for pre-commit. Would it be possible to instead add this to something like pyproject.toml or .blackrc or similar so that it would be applied e.g. if I use black from my editor?

Would it make sense to add a dev-dependency for black as well?

The only option I changed from the default is to tell Black not to change single-quote ' to double-quote ". This is what the --skip-string-normalization does.

It's been a while since I did much Python programming. If I remember correctly, there is no real difference between single-quote and double-quote strings. And there is not definite rule about what to prefer. I assume that black will not change quotes unless it is really possible; e.g., double-quote strings that include single-quotes should not be changed. And probably not f-strings and similar?

2. The PR adds pre-commit to the development tool set. I really like pre-commit. It makes it easy to lint the code on CI and enforce the coding standards.

I'm actually not a fan of commit hooks. I want to run my linting and formatters manually on my own terms (e.g. in my editor). I'm curious, is this stuff opt-in or opt-out?

Also, would it make sense to add a dev-dependency for pre-commit?

3. The PR adds a Github Action to lint the code and run the tests on different platforms.

Cool, thanks! That looks very good to me!

I realize the PR might be a bit too opinionated on the choices, so do let me know if would like to have something different and I will gladly adapt.

I think it is mostly good, thanks!

---

I have two more comments to the changes:

• The commit "feat: run pre-commit on all files" should be "chore: ..."; it does not contain any changes except clean up.
• The GitHub CI stuff: Is it possible to make it complain if someone should forget to format with pre-commit / black?
• Finally, since this is still a relatively small project, I would still like to have the liberty of pushing directly to the main branch. Thus, it could be useful to run the CI pipeline also on commits to main. Is that possible?

[ckp95]

I prefer double quotes over singles, but I can't really give a reason other than "black does it, I believe it, that settles it"

imo the point of black is that you don't have to configure it; it provides a Schelling Point so that devs don't have to argue about formatting style. If you start making amendments to black's style then you open the road to re-litigating everything and the benefit is lost.

I also agree with @lervag about not liking pre-commit hooks, imo they're too fussy and add needless friction to local development, and you can get the same benefit from CI checks. It's better to make the merge conditional on the various linter checks passing in CI, but otherwise make no restriction on what can be committed to a branch.

[lervag]

Cool; then it seems a good course of action is:

• Avoid the commit hooks (if you want them, you can always add them locally)
• Instead add a GitHub action that fails if the code fails the black formatting style
• Let's use black with the pure defaults

What do you think, @denismaciel?

[denismaciel]

That sounds good! I will adapt/simplify the PR tomorrow or the day after!

[lervag]

Great; looking forward to it :)

[denismaciel]

@lervag @ckp95

I have adapted to the suggestions above:

• Black with pure defaults
• Removal of pre-commit
• Github Action fails if code is not fully formatted with Black (that was the case before already)

The commit history is messy right now. At work, we squash before merging, so I don't have too much practice editing commits to make them look nice.

Is it fine if I just squash them into one thing and fore-push it?

Let me know if you'd like to have something else addressed in this PR.

Cheers!

[lervag]

Thanks! I took the liberty of cleaning the commits by changing the order, squashing some of them and rewording slightly. I hope you don't mind.