Adding pre-commit hook

[surbhigoel77]

• Added new file pre-commit-config.yaml with ruff as a pre-commit hook.
• Updated pyproject.toml to install pre-commit as a dependency.
• Updated README.md with installation instructions for pre-commit package

[jatkinson1000]

Hi @surbhigoel77, I think we talked about this a few times and discussed how a pre-commit workflow may be useful for those with good SE skills, but can be a burden or deterrent for those who are not so familiar with these concepts.

There is also the fact that it changes commit history on the remote, not local, easily leading to divergent histories.

My recollection was that we decided not to implement pre-commit for this project, but perhaps there was a discussion with @TomMelt or the collaborators that I missed.

[surbhigoel77]

Hi @surbhigoel77, I think we talked about this a few times and discussed how a pre-commit workflow may be useful for those with good SE skills, but can be a burden or deterrent for those who are not so familiar with these concepts.

There is also the fact that it changes commit history on the remote, not local, easily leading to divergent histories.

My recollection was that we decided not to implement pre-commit for this project, but perhaps there was a discussion with @TomMelt or the collaborators that I missed.

Thanks for your comments @jatkinson1000. We (you and me) had a conversation on slack a couple of weeks back about including some CI in the project (not Github actions). So, I included running ruff as a pre-commit hook.

The user doesn't have to do anything manually, other than correcting some minor formatting issues that ruff highlights at the time of committing changes.

We can skip it if you suggest so.

[TomMelt]

@jatkinson1000 , there might be some confusion here. I might be wrong, but I think this is just a pre-commit hook.

@surbhigoel77 , am I right in thinking that this just runs locally on the users computer, like this. That is, can you confirm that it does not run on GitHub as an action.

It might be a good idea @surbhigoel77 (assuming I understood correctly) to provide some instructions on how to setup/install the pre-commit hook and how to run it... and maybe also expected output 👍

[surbhigoel77]

@jatkinson1000 , there might be some confusion here. I might be wrong, but I think this is just a pre-commit hook.

@surbhigoel77 , am I right in thinking that this just runs locally on the users computer, like this. That is, can you confirm that it does not run on GitHub as an action.

It might be a good idea @surbhigoel77 (assuming I understood correctly) to provide some instructions on how to setup/install the pre-commit hook and how to run it... and maybe also expected output 👍

@TomMelt you are right , it's a hook that runs locally. I will update the README that mentions pre-commit hook. But the user doesn't really have to do anything. The install command is included in pyproject.toml

[TomMelt]

Ahh, but I think it does both. It also looks like it does automatically format the code...

Or am I mistaken 😕 ?

Can you confirm what this workflow does.

[TomMelt]

Wait ... sorry my bad... the lint CI already existed before this PR. It is this file

[jatkinson1000]

Thanks @surbhigoel77

Couple of comments:

• Using zsh I had to do pip install '.[pre-commit]' to escape the square brackets. Might be worth pointing this out as quite a few people are on mac. See https://stackoverflow.com/a/30539963
• I did , then pip install .[pre-commit] which performs the ruff check, but then if I want to use ruff it was not installed. Perhaps ruff should be added as a dependency to the [pre-commit] option?

Otherwise looks good.

[surbhigoel77]

• I did , then pip install .[pre-commit] which performs the ruff check, but then if I want to use ruff it was not installed. Perhaps ruff should be added as a dependency to the [pre-commit] option?

Otherwise looks good.

@jatkinson1000 I have updated the pip install .[pre-commit] to pip install pre-commit as the use of pre-commit install command makes the use of these square brackets redundant. The command pre-commit install automatically installs all the hooks that we define in 'pre-commit-config.yaml'. So ruff is automatically installed when you run this command. I have this command in README.md but will highlight that ruff need not be installed separately.

Hence the same commands can be used by both Mac and non-mac users

[jatkinson1000]

I just did a clean install and ran:

python3 -m venv venv
source venv/bin/activate
pip install .
pip install pre-commit
pre-commit install

running pip list then gives:

Package           Version
----------------- -----------
certifi           2024.8.30
cfgv              3.4.0
cftime            1.6.4
contourpy         1.3.0
cycler            0.12.1
distlib           0.3.8
filelock          3.16.1
fonttools         4.53.1
fsspec            2024.9.0
identify          2.6.1
Jinja2            3.1.4
kiwisolver        1.4.7
MarkupSafe        2.1.5
matplotlib        3.9.2
mpmath            1.3.0
netCDF4           1.7.1.post2
networkx          3.3
newCAM_emulation  0.0.0
nodeenv           1.9.1
numpy             2.1.1
packaging         24.1
pandas            2.2.2
pillow            10.4.0
pip               24.0
platformdirs      4.3.6
pre-commit        3.8.0
pyparsing         3.1.4
python-dateutil   2.9.0.post0
pytz              2024.2
PyYAML            6.0.2
scipy             1.14.1
setuptools        75.1.0
six               1.16.0
sympy             1.13.3
torch             2.4.1
torchvision       0.19.1
typing_extensions 4.12.2
tzdata            2024.1
virtualenv        20.26.5
xarray            2024.9.0

and running which ruff gives ruff not found.

Please can you check this in a clean virtual environment so we can see if it is just my machine?
Might also be worth checking you don't have ruff installed globally by running which ruff when the venv is deactivated.

[jatkinson1000]

Ah, OK, I think that works, and avoids the zsh/mac users issue.
I still think we might want contributors to be able to run ruff locally and not just rely on the pre-commit - especially if there are more complicated linting changes.

So perhaps we need to point that out or explain how they can get ruff in their local install.
there is an optional set of dependencies we already have in the pyproject.toml file ([lint]) that would do this.

It's tricky to explain in words here are there are overlapping things, so let me know if you want to discuss.

[surbhigoel77]

@jatkinson1000 I have added a format hook for ruff in the pre-commit-config.yaml file. This will allow fixing the general formatting issues in the code at the time of committing.

[jatkinson1000]

Thanks @surbhigoel77 This all seems sensible as discussed in-person.