Skip to content

Conversation

@jhlegarreta
Copy link
Contributor

Test dataset classes' optional parameters' integration with estimator.

@jhlegarreta jhlegarreta force-pushed the tst/data-optionals-integration branch from 22fd0ad to 3139500 Compare November 8, 2025 14:50
@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Nov 8, 2025

This PR was motivated by issue #223. It may seem extremely verbose and an overkill to test all this for the estimator, obut the estimator has a careful design that has some complexity and does a lot of things that seem hard to be outsourced. I am happy to transfer this to another testing module if necessary.

Although not apparent in the CI results due to a first error execution halt, running them locally surfaces a design problem related to the use of b0s: the DKI model requires multi-shell data and DIPY's internal check considers the b0s for the computation. As NiFreeze masks b0s, and thus, passing b2000 and b3000 is not considered enough.

So, related to issue #79 and PR #82.

The below check, which reproduces the DIPY logic, is put in the test so that the test can safely skip the single-shell cases:
https://github.com/nipreps/nifreeze/pull/313/files#diff-3f12d72fb4f7251c22997c345d4866aa01b84aef3c94298248a94749515ff6f3R412-R419

I can split the DKI test into a separate PR until somebody takes the time to address this, and in order to move forward with the rest.

Test dataset classes' optional parameters' integration with estimator.
@jhlegarreta jhlegarreta force-pushed the tst/data-optionals-integration branch from 3139500 to fbbc3fe Compare November 14, 2025 08:13
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.

1 participant