search

nipreps / sdcflows

Susceptibility Distortion Correction (SDC) workflows for EPI MR schemes
https://www.nipreps.org/sdcflows
Apache License 2.0
31 stars 24 forks source link

RF: Add sanitized_id field to FieldmapEstimation #444

Closed effigies closed 3 months ago

effigies commented 3 months ago

Replaces #434 along the lines described in https://github.com/nipreps/sdcflows/issues/295#issuecomment-2151041189.

I think probably the cleanest way forward for downstream is for functions to always store and pass the estimator object, and select bids_id or sanitized_id only when a string is needed and what kind of string is needed is known.

TBH, this is a needless headache that is going to invite bugs, and I think we should put a check in the validator warning about B0Field* that contains non-alphanumeric characters, including underscores.

effigies commented 3 months ago

@mgxd Test failures resolved, new tests added.

effigies commented 3 months ago

WDYT about this being in a patch release vs a feature release?

tsalo commented 3 months ago

I'd consider it a patch, since SDCFlows doesn't currently work with what the BIDS spec allows.

mgxd commented 3 months ago

I guess this will cause significant cache invalidation (at least in fmriprep) for anyone using a non-sanitized B0FieldIdentifiers, so minor release feels safer

effigies commented 3 months ago

Okay. We'll just bump to 2.9.