pre-commit hooks for Cookbooks?

[brian-rose]

I'm confused about the current state of our reliance on pre-commit for code quality.

In Foundations we have pre-commit.ci running on every PR. This service runs as a GitHub App linked to the Foundations repo.

I don't think we've ever turned this on for any Cookbook repos.

However as of #103 the Template repo has a .pre-commit-config.yaml, and pre-commit was run locally once on the repo.

All subsequent copies of the Template have the .pre-commit-config.yaml file included, but we're using it for anything.

It would be great to get this cleaned up for consistency. I see three options:

1. Get rid of the .precommit-config.yaml file, don't enforce any code style
2. Add guidance to our Contributor's Guide on how to install and run pre-commit locally on a Cookbook repo
3. Switch on the pre-commit.ci service for all existing Cookbook repos, and add guidance on how to do this for new Cookbooks.

Option 3 results in the strictest adherence to consistent code quality and formatting, with some potential for user confusion.

Option 1 is the easiest.

Option 2 is a wishy-washy bad compromise in my opinion.

@ProjectPythia/infrastructure thoughts?

[brian-rose]

Maybe there's a better compromise:

4. Switch on pre-commit.ci only for the Template repo. This will at least enforce some style guidance on future changes to the Template, which will avoid propagating poor style into future new Cookbooks -- but spare new Cookbook authors from dealing with pre-commit at all.

I think this was the intent with #103 but we never followed through and didn't switch on the service.

[ahuang11]

I think cookbook template pre-commit is failing:

An unexpected error has occurred: CalledProcessError: command: ('/Users/ahuang/.cache/pre-commit/repogb5xyvpd/py_env-python3.11/bin/python', '-mpip', 'install', '.')
return code: 1
stdout:
    Processing /Users/ahuang/.cache/pre-commit/repogb5xyvpd
      Installing build dependencies: started
      Installing build dependencies: finished with status 'done'
      Getting requirements to build wheel: started
      Getting requirements to build wheel: finished with status 'done'
      Preparing metadata (pyproject.toml): started
      Preparing metadata (pyproject.toml): finished with status 'error'
stderr:
      error: subprocess-exited-with-error

      × Preparing metadata (pyproject.toml) did not run successfully.
      │ exit code: 1
      ╰─> [17 lines of output]
          Traceback (most recent call last):
            File "/Users/ahuang/.cache/pre-commit/repogb5xyvpd/py_env-python3.11/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
              main()
            File "/Users/ahuang/.cache/pre-commit/repogb5xyvpd/py_env-python3.11/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
              json_out['return_val'] = hook(**hook_input['kwargs'])
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/Users/ahuang/.cache/pre-commit/repogb5xyvpd/py_env-python3.11/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 149, in prepare_metadata_for_build_wheel
              return hook(metadata_directory, config_settings)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/private/var/folders/8h/mk8mz7qn3_325ztm7pgb4ld00000gp/T/pip-build-env-ptgrtmca/overlay/lib/python3.11/site-packages/poetry/core/masonry/api.py", line 41, in prepare_metadata_for_build_wheel
              poetry = Factory().create_poetry(Path(".").resolve(), with_groups=False)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/private/var/folders/8h/mk8mz7qn3_325ztm7pgb4ld00000gp/T/pip-build-env-ptgrtmca/overlay/lib/python3.11/site-packages/poetry/core/factory.py", line 58, in create_poetry
              raise RuntimeError("The Poetry configuration is invalid:\n" + message)
          RuntimeError: The Poetry configuration is invalid:
            - [extras.pipfile_deprecated_finder.2] 'pip-shims<=0.3.4' does not match '^[a-zA-Z-_.0-9]+$'

          [end of output]

      note: This error originates from a subprocess, and is likely not a problem with pip.
    error: metadata-generation-failed

    × Encountered error while generating package metadata.
    ╰─> See above for output.

    note: This is an issue with the package mentioned above, not pip.
    hint: See above for details.
Check the log at /Users/ahuang/.cache/pre-commit/pre-commit.log

[brian-rose]

I got that same error locally. It was resolved by updating some package versions in the .pre-commit-config.yaml file in #121.

[brian-rose]

For now I've chosen to go with option 4. pre-commit.ci is now running on this repo. Some of the cookbook repos have an out-of-date .pre-commit-config.yaml that should be updated. Turning pre-commit.ci on for those repos would mean that auto-update PRs get generated, but may also cause confusion.

We can come back to this later!

[brian-rose]

We discussed some pros and cons of running pre-commit on cookbook repos at today's IWG.

The main pro argument is for ease of maintenance. Sticking to a consistent code style should lower the barrier for other folks aside from the original author to help with maintenance of notebooks in cookbooks.

The main con is the additional complexity and consequent need to write more guidance for cookbook authors.

For now we are taking no action but leaving the above compromise (option 4) in place.