Devops: Install from abstract dependencies for pre-commit CI #6368
Conversation
The workflow running pre-commit was installing from the requirements before installing `pip install .[pre-commit]`. The install from the requirements file was added because some hooks would fail due to import errors. The proper solution is simply to install all extras whose dependencies are imported during pre-commit checks.
sphuber
force-pushed
the
fix/6366/pre-commit-dependencies
branch
2 times, most recently
from
e2689b1 to
fcfffdd
Compare
.github/workflows/ci-style.yml
Outdated
| - name: Install python dependencies | ||
| uses: ./.github/actions/install-aiida-core | ||
| with: | ||
| python-version: '3.10' | ||
| extras: '[pre-commit]' | ||
| from-requirements: 'true' | ||
| extras: '[atomic_tools,pre-commit,rest,tests,tui]' |
There was a problem hiding this comment.
I was thinking about this and I found this solution really unintuitive and ran into this problem many times locally.
As a developer, I would expect that if I install pre-commit extras, the pre-commit checks shoud simply pass, instead of having to figure out essentially manually which other extras I need...
There was a problem hiding this comment.
Could we instead tell mypy to install additional dependencies (e..g type stubs), like we do in aiidalab-launch? Although I guess i am not sure how to ensure the correct versions are installed?
There was a problem hiding this comment.
Another option, seems like since pip 21.2, one could define extras recursively, so [pre-commit] could transitively include all the needed extras?
https://hynek.me/articles/python-recursive-optional-dependencies/
There was a problem hiding this comment.
As a developer, I would expect that if I install pre-commit extras, the pre-commit checks shoud simply pass, instead of having to figure out essentially manually which other extras I need...
Fair enough, but I really didn't want to duplicate requirements in pre-commit just because they need to be importable.
Could we instead tell mypy to install additional dependencie
We could, but that would require again duplicating requirements which risk running out of sync, and it is not just mypy that would need this.
Another option, seems like since pip 21.2, one could define extras recursively, so [pre-commit] could transitively include all the needed extras?
That is great actually. If that also works with uv, that'd be a great solution as far as I am concerned.
Co-authored-by: Daniel Hollas <[email protected]>
Fixes #6366
The workflow running pre-commit was installing from the requirements before installing
pip install .[pre-commit]. The install from the requirements file was added because some hooks would fail due to import errors. The proper solution is simply to install all extras whose dependencies are imported during pre-commit checks.