-
Notifications
You must be signed in to change notification settings - Fork 42
REF: generalised anat references 2.0 #477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
REF: generalised anat references 2.0 #477
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had a chance to jot down some thoughts - i like the overall direction, but we can take this moment to clean up the logic (which admittedly is a bit of a mess if derived from nibabies 😅 )
'data': subject_data[modality], | ||
'n': len(subject_data[modality]), | ||
'precomputed': f'{modality}_preproc' in precomputed, | ||
'role': 'reference' if modality.capitalize() == reference_anat else 'aux', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this just be a bool, since if not we will treat as auxillary
'role': 'reference' if modality.capitalize() == reference_anat else 'aux', | |
'is_ref': modality.capitalize() == reference_anat, |
anat_inputs = { | ||
modality: { | ||
'data': subject_data[modality], | ||
'n': len(subject_data[modality]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt about
'n': len(subject_data[modality]), | |
'count': len(subject_data[modality]), |
'role': 'reference' if modality.capitalize() == reference_anat else 'aux', | ||
} | ||
for modality in ['t1w', 't2w', 'flair'] | ||
if modality in subject_data.keys() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since i believe all keys are in subject_data, this will check if the list is not empty
if modality in subject_data.keys() | |
if subject_data.get(modality) |
@@ -80,8 +82,7 @@ def test_init_anat_preproc_wf( | |||
hires=False, | |||
longitudinal=False, | |||
msm_sulc=False, | |||
t1w=[str(bids_root / 'sub-01' / 'anat' / 'sub-01_T1w.nii.gz')], | |||
t2w=[str(bids_root / 'sub-01' / 'anat' / 'sub-01_T2w.nii.gz')], | |||
anat=collect_anat(collect_data(bids_root, '01')[0], {}, 'T1w'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pretty noisy and has a lot going on for one line
anat_inputs[reference_anat.lower()].update( | ||
{ | ||
f'have_{preproc}': f'{reference_anat.lower()}_{preproc}' in precomputed | ||
for preproc in ['mask', 'tpms', 'dseg'] | ||
} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this, i wonder if this precomputed parsing would be better suited for a separate function
After pushing #475 , @mgxd suggested to move the logic to its own workflow.
As a result, we have a new function,
collect_anat
which will assign a modality as either 'reference' or 'aux'. At the moment, 'reference' is hard-coded to be T1w but this provides an opportunity for command line overrides or alternative pipelines for nirodents/nibabies