Skip to content

Adopt NiPreps-style packaging#1028

Merged
tsalo merged 49 commits intomasterfrom
packaging-parity
Apr 13, 2026
Merged

Adopt NiPreps-style packaging#1028
tsalo merged 49 commits intomasterfrom
packaging-parity

Conversation

@tsalo
Copy link
Copy Markdown
Member

@tsalo tsalo commented Mar 4, 2026

Closes none.

Changes proposed in this pull request

  • Move FSL and Connectome Workbench requirements into Dockerfile.
  • Add Dockerfile.base with non-Python, non-conda requirements. This still builds off of the package-wise Docker images in qsiprep_build, except for the main Dockerfile there.
  • Add pixi.lock file.
  • Add pixi-lock GitHub Action.
  • Stop using pkg_resources, which is no longer supported. Use importlib.resources instead.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 59.49367% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.13%. Comparing base (4f38efc) to head (af97b4a).

Files with missing lines Patch % Lines
qsiprep/interfaces/ants.py 7.14% 13 Missing ⚠️
qsiprep/interfaces/itk.py 9.09% 10 Missing ⚠️
qsiprep/utils/resources.py 40.00% 2 Missing and 1 partial ⚠️
qsiprep/interfaces/gradients.py 75.00% 1 Missing and 1 partial ⚠️
qsiprep/interfaces/eddy.py 66.66% 1 Missing ⚠️
qsiprep/workflows/anatomical/volume.py 88.88% 1 Missing ⚠️
qsiprep/workflows/dwi/intramodal_template.py 66.66% 1 Missing ⚠️
qsiprep/workflows/fieldmap/pepolar.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1028      +/-   ##
==========================================
+ Coverage   46.28%   47.13%   +0.85%     
==========================================
  Files          65       66       +1     
  Lines        9814     9836      +22     
  Branches     1088     1084       -4     
==========================================
+ Hits         4542     4636      +94     
+ Misses       5036     4979      -57     
+ Partials      236      221      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tsalo tsalo requested a review from mattcieslak March 11, 2026 17:34
@tsalo
Copy link
Copy Markdown
Member Author

tsalo commented Mar 11, 2026

It's finally ready!!!!

Copy link
Copy Markdown
Contributor

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.

Pull request overview

This PR modernizes QSIPrep’s packaging/build/CI to align with NiPreps-style workflows, including moving away from pkg_resources, adopting pixi environments for container builds, and reworking CI to build/test via Docker images.

Changes:

  • Replace pkg_resources.resource_filename(...) with importlib.resources.files(...)-based resource resolution across workflows/interfaces.
  • Introduce pixi-based build/test Docker images (plus a new Dockerfile.base) and update CircleCI to build/test using those images and cached datasets.
  • Add lockfile automation (pixi-lock.yml), new maintenance docs, and expand test coverage (incl. a new gradients unit test).

Reviewed changes

Copilot reviewed 29 out of 31 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
qsiprep/workflows/fieldmap/unwarp.py Switch resource file lookup away from pkg_resources.
qsiprep/workflows/fieldmap/syn.py Switch atlas/transform resource lookup away from pkg_resources.
qsiprep/workflows/fieldmap/pepolar.py Switch ANTs settings JSON resource lookup away from pkg_resources.
qsiprep/workflows/dwi/util.py Update commented resource lookup example to new files()/as_path() pattern.
qsiprep/workflows/dwi/registration.py Replace pkg_resources resource lookup with files()/as_path().
qsiprep/workflows/dwi/intramodal_template.py Replace pkg_resources resource lookup with files()/as_path().
qsiprep/workflows/dwi/hmc.py Replace pkg_resources resource lookup with files()/as_path().
qsiprep/workflows/dwi/fsl.py Replace pkg_resources lookup for eddy config JSON with files()/as_path().
qsiprep/workflows/anatomical/volume.py Replace pkg_resources lookups for LUT/config/JSON/identity transform resources.
qsiprep/utils/resources.py Add as_path() helper to adapt importlib.resources.files() results into filesystem paths.
qsiprep/tests/test_interfaces_gradients.py Add an end-to-end unit test for get_fsl_motion_params().
qsiprep/tests/test_cli.py Adjust integration tests (dataset choice, sloppy mode, and add “unbiased” anat-only run).
qsiprep/tests/run_local_tests.py Update local docker test runner to match pixi-based image layout.
qsiprep/tests/data/intramodal_template_outputs.txt Update expected outputs list for intramodal template integration test.
qsiprep/interfaces/itk.py Make composite transform disassembly more robust across ANTs/ITK naming conventions.
qsiprep/interfaces/gradients.py Harden ITK→FSL conversion by using subprocess.run and text ITK transforms to avoid segfaults.
qsiprep/interfaces/eddy.py Replace pkg_resources lookup for b02b0_1.cnf with files()/as_path().
qsiprep/interfaces/ants.py Make output transform discovery more robust across ANTs naming/location conventions.
qsiprep/cli/workflow.py Replace pkg_resources lookups; update pandoc invocation to use --citeproc.
pyproject.toml Update key dependencies; add pixi workspace configuration; move codespell config into pyproject.toml.
Dockerfile.base Add new “base” image Dockerfile for non-Python runtime dependencies.
Dockerfile Switch to pixi-based environment build; produce test and qsiprep image targets.
.maint/INSTRUCTIONS.md Add maintainer docs for Docker/pixi/tag bump workflows.
.gitignore Ignore pixi environments and Cursor env file.
.github/workflows/pixi-lock.yml Add a lockfile-update workflow for PRs.
.gitattributes Treat pixi.lock as generated/binary for merges and linguist.
.dockerignore Ensure .pixi is excluded from docker build context.
.cursor/rules/cursorenv.mdc Add Cursor editor rule for conda environment activation.
.codespellrc Remove standalone codespell config (now in pyproject.toml).
.circleci/config.yml Major CI refactor: build images + run tests inside docker with cached data/registry and combined coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread qsiprep/utils/resources.py
Comment thread pyproject.toml
Comment thread pyproject.toml
Comment thread Dockerfile.base
Comment on lines +59 to +62
ENV PERL5LIB="$MINC_LIB_DIR/perl5/5.8.5" \
MNI_PERL5LIB="$MINC_LIB_DIR/perl5/5.8.5" \
PATH="$FREESURFER_HOME/bin:$FSFAST_HOME/bin:$FREESURFER_HOME/tktools:$MINC_BIN_DIR:$PATH" \
FREESURFER_DEPS="bc ca-certificates curl libgomp1 libxmu6 libxt6 tcsh perl"
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

PATH references $FSFAST_HOME, but that variable is never set in this Dockerfile, so the value will expand to empty at build time (and triggers the UndefinedVar warning noted in the PR description). Either define FSFAST_HOME explicitly or remove it from PATH if it’s not required.

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/pixi-lock.yml
Comment thread Dockerfile
Comment thread qsiprep/tests/test_interfaces_gradients.py Outdated
Comment thread .circleci/config.yml
tsalo and others added 3 commits March 12, 2026 10:08
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@mattcieslak mattcieslak left a comment

Choose a reason for hiding this comment

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

Amazing, this looks great

Comment thread .cursor/rules/cursorenv.mdc Outdated
Comment thread qsiprep/tests/test_cli.py
@@ -446,34 +450,80 @@ def test_multi_t1w(data_dir, output_dir, working_dir):

This tests the following features:
- freesurfer's robust template
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we actually use freesurfer's robust template anymore

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think a lot of the docstrings for the integration tests are outdated/incorrect. We should probably update them at some point.

Comment thread qsiprep/tests/test_cli.py
@tsalo
Copy link
Copy Markdown
Member Author

tsalo commented Apr 13, 2026

@mattcieslak since you've approved, I'll merge this once CI passes. That alright?

@mattcieslak
Copy link
Copy Markdown
Collaborator

Sounds great! I think we may want to switch the main branch to main at some point too, since all the other apps use it

@tsalo tsalo merged commit 7e9be23 into master Apr 13, 2026
20 checks passed
@tsalo tsalo deleted the packaging-parity branch April 13, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants