Skip to content

ENH: Use pixi to install pre-commit, python for pre-commit#5032

Merged
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:masterfrom
thewtex:pre-commit-pixi
Dec 11, 2024
Merged

ENH: Use pixi to install pre-commit, python for pre-commit#5032
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:masterfrom
thewtex:pre-commit-pixi

Conversation

@thewtex
Copy link
Member

@thewtex thewtex commented Dec 10, 2024

On Windows, Python is not available by default. The SetupForDevelopment.sh script does not complete successfully in a default build on Windows.

This uses pixi to install pre-commit, python for pre-commit, so the contribution experience works out-of-the-box for experienced and new contributors to ITK.

This also improves the isolation and maintainability of pre-commit tooling. Everything needed for a commit is local to the ITK repository. We also get more reproducible versions of Python and pre-commit via pixi's configuration in pyproject.toml and pixi.lock.

pixi is a self-contained, single-file, cross-platform Rust binary. A local version is installed to .git/hooks/pixi/bin/pixi. Pixi's python and pre-commit is installed to .pixi/.

The installation of tools is:

SetupForDevelopment.sh -> curl -> pixi -> python, pre-commit -> pre-commit's hook dependencies

This is to help get @N-Dekker going again and make the contribution process easier for new contributors.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 4 changed files in this pull request and generated no suggestions.

Files not reviewed (2)
  • Utilities/GitSetup/setup-precommit: Language not supported
  • Utilities/SetupForDevelopment.sh: Language not supported

@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation labels Dec 10, 2024
@thewtex
Copy link
Member Author

thewtex commented Dec 10, 2024

This also makes it easy to run the pre-commit hooks on demand on all files via:

pixi run pre-commit-run

@thewtex
Copy link
Member Author

thewtex commented Dec 10, 2024

Tested locally on Mac, Linux, and Windows.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it, and it works.

@dzenanz
Copy link
Member

dzenanz commented Dec 10, 2024

clang-format changes 4 files for me when I run pixi run pre-commit-run:

Modules/Core/Common/include/itkQuadrilateralCell.hxx
Modules/Core/Common/include/itkSymmetricEigenAnalysis.hxx
Modules/Filtering/QuadEdgeMeshFiltering/include/itkParameterizationQuadEdgeMeshFilter.hxx
Modules/IO/JPEG2000/src/itkJPEG2000ImageIO.cxx

Does that happen for you? If so, can you add these changes as a new commit?

Copy link
Member

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have not tried, but I trust the change. Thanks @thewtex 💯.

On Windows, Python is not available by default. The
SetupForDevelopment.sh script does not complete successfully in a
default build on Windows.

This uses pixi to install pre-commit, python for pre-commit, so the
contribution experience works out-of-the-box for experienced and new
contributors to ITK.

This also improves the isolation and maintainability of pre-commit
tooling. Everything needed for a commit is local to the ITK repository.
We also get more reproducible versions of Python and pre-commit via
pixi's configuration in pyproject.toml and pixi.lock.

pixi is a self-contained, single-file, cross-platform Rust binary. A local version is
installed to .git/hooks/pixi/bin/pixi. Pixi's python and pre-commit is
installed to .pixi/.

The installation of tools is:

  SetupForDevelopment.sh -> curl -> pixi -> python, pre-commit -> pre-commit's hook dependencies

This also makes it easy to run the pre-commit hooks on demand on all
files via:

```
pixi run pre-commit-run
```
@thewtex
Copy link
Member Author

thewtex commented Dec 10, 2024

Does that happen for you? If so, can you add these changes as a new commit?

@dzenanz thanks for testing. Yes -- I will add the commit to this branch.

Generated via:

  pixi run pre-commit-run
@github-actions github-actions bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module labels Dec 10, 2024
Copy link
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Matt!

@hjmjohnson hjmjohnson merged commit b19aa1f into InsightSoftwareConsortium:master Dec 11, 2024
@thewtex thewtex deleted the pre-commit-pixi branch December 11, 2024 11:50
@dzenanz
Copy link
Member

dzenanz commented Dec 13, 2024

There seems to be some unforeseen side-effects of pre-commit hooks. Linting now fails for remote modules with message clang-format version 19.1.x is required.

@blowekamp
Copy link
Member

Likely the action needs to be updated, may be something like this: https://github.com/InsightSoftwareConsortium/ITKClangFormatLinterAction/pull/3/files but with the correct version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants