Skip to content

Move pre-commit in dev for automatic install#1366

Merged
esoteric-ephemera merged 3 commits intomaterialsproject:mainfrom
thomasloux:fix/precommit-dev-deps
Jan 2, 2026
Merged

Move pre-commit in dev for automatic install#1366
esoteric-ephemera merged 3 commits intomaterialsproject:mainfrom
thomasloux:fix/precommit-dev-deps

Conversation

@thomasloux
Copy link
Contributor

Summary

In current installation, you need to run uv sync --extra dev to use pre-commit. I've just installed it using uv add --dev pre-commitso that this dev group is automatically installed when running uv sync.

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request
title.

Before a pull request can be merged, the following items must be checked:

  • Code is in the standard Python style.
    The easiest way to handle this is to run the following in the correct sequence on
    your local machine. Start with running ruff and ruff format on your new code. This will
    automatically reformat your code to PEP8 conventions and fix many linting issues.
  • Doc strings have been added in the Numpy docstring format.
    Run ruff on your code.
  • Type annotations are highly encouraged. Run mypy to
    type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more
efficient if you already fix most errors prior to submitting the PR. It is highly
recommended that you use the pre-commit hook provided in the repository. Simply run
pre-commit install and a check will be run prior to allowing commits.

@JaGeo
Copy link
Member

JaGeo commented Jan 2, 2026

@thomasloux Thank you. I am not a pre-commit expert. Is the version update stricly required?

@thomasloux
Copy link
Contributor Author

No, it just updated by running uv add --dev pre-commit. But pre-commit runs fine with the new version. I can roll back to the same version to prevent error.

@JaGeo
Copy link
Member

JaGeo commented Jan 2, 2026

I see. Thanks. I think we can keep the old version.

Could you check the documentation for the pre-commit installation and update this one?

@thomasloux
Copy link
Contributor Author

What do you want me to change regarding pre-commit? There is nothing about pre-commit in the docs as far as I know: https://materialsproject.github.io/atomate2/about/contributing.html

@JaGeo
Copy link
Member

JaGeo commented Jan 2, 2026

It is here: https://materialsproject.github.io/atomate2/dev/dev_install.html

Can you add the uv options and check if all pip ones are updated? That would be great!

@esoteric-ephemera
Copy link
Collaborator

esoteric-ephemera commented Jan 2, 2026

Just noting: we don't use uv sync in CI (see the tests workflow), you can also just uv pip install atomate2[dev] to pick up these extra development-only requirements

Not sure this needs to be modified

@thomasloux
Copy link
Contributor Author

This does not need to be updated. I was mainly suggesting an alternative configuration that I find more convenient. This is independent of the CI workflow.

@thomasloux
Copy link
Contributor Author

@esoteric-ephemera in CI you can also replace 'pip install .[extra1, extra2]' with 'uv sync --extra extra1 --extra extra2' if you have uv installed and want to only use uv and drop pip.

@esoteric-ephemera
Copy link
Collaborator

Thanks! We should probably make a decision regarding using uv more completely with atomate2 but that can come in a separate fix, will merge this in for now

@esoteric-ephemera esoteric-ephemera enabled auto-merge (rebase) January 2, 2026 17:47
@JaGeo
Copy link
Member

JaGeo commented Jan 2, 2026

@esoteric-ephemera I think this would be great! Also, adding this to our documentation would be good

@esoteric-ephemera esoteric-ephemera merged commit 77e0660 into materialsproject:main Jan 2, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants