Add pre-commit config and GitHub Actions job

[nicholasjng]

Contains the following hooks:

• buildifier - for formatting and linting Bazel files.
• mypy, ruff, isort, black - for Python typechecking, import hygiene, static analysis, and formatting.

The pylint CI job was changed to be a pre-commit CI job, where pre-commit is bootstrapped via Python.

Pylint is currently no longer part of the code checks, but can be re-added if requested. The reason to drop was that it does not play nicely with pre-commit, and lots of its functionality and responsibilities are actually covered in ruff.

[dmah42]

looks like pre-commit needs installing.

[nicholasjng]

This is ready for re-review. Not sure why the buildifier fails in CI, it passes on my machine (TM).

Re: integrating clang-format with pre-commit, it seems quite easy, in case you want to restructure / unify the actions: https://stackoverflow.com/questions/55965712/how-do-i-add-clang-formatting-to-pre-commit-hook

EDIT: Need to rebase, one second.

[dmah42]

"pylint" is not installed i think

[nicholasjng]

Yeah, I forgot deleting that - most (but not all) of pylint's work is taken over by ruff, is it ok with you to drop pylint then? (See the PR comment for my reasons)

I also forgot to add the tool configs to the pyproject.toml - after doing so, I got 31 flags in the report.py, so I am strongly considering excluding that file from typechecking (some crazy stuff is happening in there, which I don't feel like fixing atm).

Barring some type annotations, the stuff I'm about to push should complete this. Are you fine with line length 100 for Python files, or do you require a specific different value?

[dmah42]

Yeah, I forgot deleting that - most (but not all) of pylint's work is taken over by ruff, is it ok with you to drop pylint then? (See the PR comment for my reasons)

if it's a swap out for ruff then sure.

I also forgot to add the tool configs to the pyproject.toml - after doing so, I got 31 flags in the report.py, so I am strongly considering excluding that file from typechecking (some crazy stuff is happening in there, which I don't feel like fixing atm).

sgtm

Barring some type annotations, the stuff I'm about to push should complete this. Are you fine with line length 100 for Python files, or do you require a specific different value?

80 please: https://google.github.io/styleguide/pyguide.html

[nicholasjng]

Link to the ruff / pylint comparison: https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-pylint

TLDR; it's not a 1:1 but it covers more rules, and especially when including mypy you more than make up for what you lose on static typechecking.

Python files are reformatted to length 80 now.

[dmah42]

Link to the ruff / pylint comparison: https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-pylint

TLDR; it's not a 1:1 but it covers more rules, and especially when including mypy you more than make up for what you lose on static typechecking.

Python files are reformatted to length 80 now.

sorry, python style is also 2 indents. that should also minimise a lot of the changes.

[nicholasjng]

~You mean 2 spaces as indent? Alright, that means I will have to remove black again (they only support 4-space indents).~

~I think I will reset and submit the configurations again, and then run the remaining hooks (i.e. excluding black) over the codebase. That should eliminate the unwanted changes.~

Looking at the style guide, it seems indentations are 4 spaces? (https://google.github.io/styleguide/pyguide.html#s3.4-indentation)

I looked at the diff of e.g. report.py, and it seems that a lot of the code is already 4-space indented - am I misunderstanding something here?

It seems to me like black formatting is conforming to the style guide here, the diff is mostly line breaks on reformats on inline lists and tuples, and JSON objects, and of course the switch from single to double quotes.

[dmah42]

~You mean 2 spaces as indent? Alright, that means I will have to remove black again (they only support 4-space indents).~

~I think I will reset and submit the configurations again, and then run the remaining hooks (i.e. excluding black) over the codebase. That should eliminate the unwanted changes.~

Looking at the style guide, it seems indentations are 4 spaces? (https://google.github.io/styleguide/pyguide.html#s3.4-indentation)

wow. that's different to the internal version. that'll be fun to integrate later 😅

I looked at the diff of e.g. report.py, and it seems that a lot of the code is already 4-space indented - am I misunderstanding something here?

no i was.

It seems to me like black formatting is conforming to the style guide here, the diff is mostly line breaks on reformats on inline lists and tuples, and JSON objects, and of course the switch from single to double quotes.

[nicholasjng]

So regarding clang-format in pre-commit yes or no, what's your take?

Style guide decision is up to you, I don't want to needlessly mess up the repository formatting. Just give me a heads-up on how this should be handled and I'll resubmit accordingly if necessary.

[dmah42]

So regarding clang-format in pre-commit yes or no, what's your take?

Style guide decision is up to you, I don't want to needlessly mess up the repository formatting. Just give me a heads-up on how this should be handled and I'll resubmit accordingly if necessary.

clang-format: not yet.

style guide: follow what the external links say please

[nicholasjng]

style guide: follow what the external links say please

https://google.github.io/styleguide/pyguide.html#s1-background says that "Many teams use the black auto-formatter" (which is configured in this PR), so does that make it eligible to use?

From a quick survey of Google Python OSS projects, https://github.com/google/yapf seems to be another popular one - that's also available for setup in pre-commit.

[dmah42]

style guide: follow what the external links say please

https://google.github.io/styleguide/pyguide.html#s1-background says that "Many teams use the black auto-formatter" (which is configured in this PR), so does that make it eligible to use?

From a quick survey of Google Python OSS projects, https://github.com/google/yapf seems to be another popular one - that's also available for setup in pre-commit.

i have no opinion about the tools for Python, as long as the styling is consistent and matches the google style i'm happy.

[nicholasjng]

This should be ready to review then. From my best-effort research, the Black style is consistent with the public Google Python Style guide.

[dmah42]

thank you!