[BUG]: Validation failure of multiple pre-processing steps with different input data types requirement #339
No reviewers
Labels
No labels
CRITICAL
Stale
WIP
bug
concept
coordinate
dataset
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
maintenance
maps
marker
mask
on hold
parcellation
preprocess
question
ready
storage
template-space
triage
wontfix
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
juaml/junifer!339
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/multi-preprocessor-validation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Is there an existing issue for this?
Current Behavior
I'm running a YAML with two pre-processing steps: confound remover + Space Warper.
The process fails because the Space Warper requires the T1w + Warp which is not outputed by the fMRIPrepConfoundRemover.
The bug is basically that: it's not accounting that the data object has the T1w and Warp objects. It just gets the output of the fMRIPrepConfoundRemover.
Expected Behavior
I would expect that the two steps can be used.
Steps To Reproduce
junifer run --element sub-0001 NAME_OF_YAMLEnvironment
Relevant log output
Anything else?
No response
Issue can be solved either at the marker collection level, by "merging" the new and old
t_datain line 140 here:github.com/juaml/junifer@e0cb0f233b/junifer/markers/collection.py (L133-L142)OR, at the PipelineStepMixin level, which I don't really recomend as it is not something of the step, but the application of subsequent steps.
Something like this works:
Can you check if the validation passes?
Codecov Report
All modified and coverable lines are covered by tests ✅
Additional details and impacted files
100.00% <ø> (ø)Flags with carried forward coverage won't be shown. Click here to find out more.
This will not work in case you have a preprocessing step that only modifies one type and passes the others through.
Basically, after pre-processing you should have AT LEAST the same elements as before pre-processing:
t_data = list(set(validated_input_data_types) | set(t_data))Yeah agree, good point, made the change.
I still think this needs to be done inside the for. What if a preprocessor adds a new type than then is used by another preprocessor?
Fair argument, have updated with the code you posted.