Replace black with ruff and add pre-commit

[davidwilby]

Hey @tom-andersson et al - just a maintenance-related PR for your perusal, feel free to reject if this isn't your thing.

What this PR does

• resolves Potential new feature: ruff linting #109
• replaces black with ruff as the linter of choice, partially because of extra features such as linting docstrings in google-style (which black may do as well, but I haven't been able to find it)
• adds pre-commit and a pre-commit config to run the ruff linter and formatter on a pre-commit hook.
• adds info to CONTRIBUTING.md detailing the use of these tools.

What this PR doesn't do

• actually formatting the codebase with ruff so far in this branch to avoid the massive diff
• add a pull request template to remind contributors to follow these steps (that will follow in another PR)

At the moment the CI check fails since ruff is checking all the docstrings, we may be able to soften the docstring rules (configured in pyproject.toml) to something we agree with (e.g. I don't necessarily think that every module needs a docstring)

FYI @nilsleh

[kallewesterling]

LGTM!

[tom-andersson]

Thank you @davidwilby, this does look quite useful. I checked the error message of the CI and we now get many failing docstring checks. Can you 1) address the simpler syntax ones 2) silence more complex issues like missing public function docstrings, so that tests pass?

It looks like we can ignore error messages in pyproject.toml with:

[tool.ruff.lint]

ignore = [
    # Relax the convention by _not_ requiring documentation for every function parameter.
    "D417",
]

[davidwilby]

@tom-andersson I've added a number of rules to ignore and performed the remaining linting of docstrings (the code itself was completely compliant from using black)

The remaining linting failures are for missing docstrings on public methods/functions:

deepsensor/data/loader.py:809:9: D102 Missing docstring in public method
deepsensor/data/task.py:32:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:446:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:469:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:492:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:517:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:553:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:588:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:620:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:638:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:977:5: D103 Missing docstring in public function
deepsensor/model/model.py:84:9: D102 Missing docstring in public method
deepsensor/model/model.py:612:5: D103 Missing docstring in public function
deepsensor/tensorflow/__init__.py:19:5: D103 Missing docstring in public function
deepsensor/torch/__init__.py:19:5: D103 Missing docstring in public function
deepsensor/train/train.py:168:9: D102 Missing docstring in public method

I've had a go at adding a few missing docstrings in 678602a but other than that I don't know enough to write them.

One option (which I've used in a couple of places) is to add a # noqa tag for functions/methods that aren't really meant for public usage.

Could you add some docstrings or advise on those that are remaining when you get the chance?

[tom-andersson]

Thanks @davidwilby and sorry for the delay. It's helpful to see where docstrings are missing, so let's keep that behaviour in the linter. I'll add a commit to this PR with the missing docstrings, aiming for this week or the next.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[davidwilby]

An update to the docstrings that need attention:

$ ruff check --output-format concise
deepsensor/data/loader.py:809:9: D102 Missing docstring in public method
deepsensor/data/task.py:32:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:453:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:476:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:499:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:524:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:560:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:595:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:631:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:667:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:699:9: D102 Missing docstring in public method
deepsensor/model/convnp.py:717:9: D102 Missing docstring in public method
Found 12 errors.

[davidwilby]

Okay, I think there has to be some statute of limitations on pull requests 😄 so I've opted to ignore these missing docstrings (except one I could actually fill in) and I'll open an issue unless you object @tom-andersson - then we can move on with our lives and merge this after nearly 6 months.

[tom-andersson]

Let's do it @davidwilby, LGTM. Sorry for blocking - time got away from me w/ my full-time job.

So to confirm, developers don't need to run ruff check before submitting, they just need to run pre-commit install and git commit will run the ruff formatter?
I added a comment about exposing the actual ruff command to run manually in the documentation. Sometimes I like to see what the formatter has done before committing.

You'll need to sync and re-run, but I think you have permissions to rebase + merge yourself?

[davidwilby]

@tom-andersson thanks for the review! I've rebased locally since there were many conflicts. All the tests are passing but I'll give you the opportunity to cast a quick eye over it and check I haven't made any mistakes in resolving the conflicts.

[tom-andersson]

Out of curiosity, what is this noqa: D102? Is this comment flag handled automatically by ruff?

[tom-andersson]

Thanks @davidwilby, took another look and passes a sanity check. I'd rather you removed the changes to the documentation notebooks before merging though (see my review comment). WDYT?