Skip to content

rf: Transition to nipost.collect_derivatives - #585

Merged
effigies merged 8 commits into
nipreps:masterfrom
effigies:nipost
Oct 26, 2026
Merged

effigies merged 8 commits into
nipreps:masterfrom
effigies:nipost

Conversation

@effigies

@effigies effigies commented Sep 8, 2026 •

Copy link
Copy Markdown
Member

This PR has been rebased on #586, and so we should be exercising all precomputed paths. It:

  • Pins feat/fix: Implement updated collection logic nipost#1, and adjusts data retrieval code.
  • Updates tests to pin nipost behavior that diverged from the smriprep collector's behavior. In particular, dropping groups of files like TPMs if all weren't present. This was added as an xfail in test: Make workflow-building tests more resilient #586 and now passes. By moving completeness checks into the collector, the workflow-generating logic is simplified.
  • Adds logging to precomputed derivatives. At INFO level, the full list of derivatives to be reused is reported at the beginning of init_anat_fit_wf(). At DEBUG level, overrides are logged while merging multiple derivatives. Combined, these give full transparency into the override behavior.

@effigies
effigies force-pushed the nipost branch 3 times, most recently from ce1fa92 to 34fa579 Compare September 9, 2026 16:35
@codecov

codecov Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.81%. Comparing base (10d70ee) to head (6934d01).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #585      +/-   ##
==========================================
+ Coverage   90.22%   90.81%   +0.58%     
==========================================
  Files          23       23              
  Lines        2077     2046      -31     
  Branches      239      226      -13     
==========================================
- Hits         1874     1858      -16     
+ Misses        144      139       -5     
+ Partials       59       49      -10     
Flag Coverage Δ
ds054 46.67% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

Comment thread src/smriprep/workflows/anatomical.py Outdated
Cover session-less collection, sphere_reg precedence across datasets and
with an incomplete fsaverage pair, and whether the fsnative registration
is estimated when its transform is missing.
Replace the io_spec.json collector with nipost.bids.collect_derivatives
and anat_spec.yml. The precomputed dictionary now nests images, surfaces
and transforms; init_anat_fit_wf reads the new shape.

sphere_reg prefers space-fsaverage and falls back to legacy naming, which
is still reported with the deprecation warning. Tissue probability maps
are only reused when all three are present.
nipost raises when a query matches more than one file, for example
derivatives from two sessions collected without a session label.
A legacy registration sphere also matches the native sphere query, and a
transform's JSON sidecar or a points-mode transform matches the transform
queries, so collection fails with an ambiguous match.
Log at DEBUG level when a later derivatives dataset replaces an earlier
one's derivative, so that later-dataset precedence is visible. When the
fit workflow is built, log one INFO summary of every precomputed
derivative it reuses.

Move the legacy sphere_reg warning into the same report, so that it is
raised for derivatives collected by any caller, and only for files that
are actually reused.
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "pixi lock",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [
  "pyproject.toml",
  "pixi.lock"
 ],
 "outputs": [
  "pixi.lock"
 ],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@effigies
effigies merged commit 327a651 into nipreps:master Oct 26, 2026
19 checks passed
@effigies
effigies deleted the nipost branch October 26, 2026 15:52
ChristianGaser added a commit to ChristianGaser/smriprep that referenced this pull request Oct 27, 2026
…g recon-all

Merging master brought in nipreps#585, which nests precomputed derivatives under
``images``, ``surfaces`` and ``transforms`` and renames ``t1w_mask`` to
``mask``.  The merge applied without conflicts, but ``needs_reconall()``,
the precomputed T2w passthrough and the CIFTI curvature still looked up the
flat layout, so recon-all was never skipped and a precomputed T2w was
dropped again.  The merge also moved the fsnative-transform assertion of
``test_anat_fit_precomputes_omit_one`` into a test without ``omit``.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ChristianGaser added a commit to ChristianGaser/fmriprep that referenced this pull request Oct 27, 2026
Without a FreeSurfer subject, ``--cifti-output`` takes the curvature from
the precomputed ``hemi-{L,R}_curv.shape.gii`` and fails at build time if
there is none.  Neither case was tested, so a lookup that misses the
curvature, as one on the flat layout does once nipreps/smriprep#585 nests
surfaces under ``surfaces``, raised that error with the curvature present
and no test failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants