From 5fee78414a1903f02b264ea66de0dd3ca27a9371 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 26 Nov 2020 16:10:19 +0100 Subject: [PATCH] enh: keep a registry of already-used identifiers (and auto-generate new) Resolves: #125. --- sdcflows/fieldmaps.py | 45 +++++++++++++++++++++++++ sdcflows/tests/test_fieldmaps.py | 56 +++++++++++++++++++++++++++++++- sdcflows/workflows/base.py | 16 ++++----- 3 files changed, 108 insertions(+), 9 deletions(-) diff --git a/sdcflows/fieldmaps.py b/sdcflows/fieldmaps.py index c2974e1d55..7879694568 100644 --- a/sdcflows/fieldmaps.py +++ b/sdcflows/fieldmaps.py @@ -9,6 +9,9 @@ from niworkflows.utils.bids import relative_to_root +_unique_ids = set() + + class MetadataError(ValueError): """A better name for a specific value error.""" @@ -59,6 +62,27 @@ def _type_setter(obj, attribute, value): return value +def _id_setter(obj, attribute, value): + """Ensure uniqueness of B0FieldIdentifier metadata.""" + if obj.bids_id: + if obj.bids_id != value: + raise ValueError("Unique identifier is already set") + _unique_ids.add(value) + return value + + if value is True: + value = f"auto_{len([el for el in _unique_ids if el.startswith('auto_')])}" + elif not value: + raise ValueError("Invalid unique identifier") + + if value in _unique_ids: + raise ValueError("Unique identifier has been previously registered.") + + _unique_ids.add(value) + return value + + + @attr.s(slots=True) class FieldmapFile: """ @@ -253,6 +277,9 @@ class FieldmapEstimation: method = attr.ib(init=False, default=EstimatorType.UNKNOWN, on_setattr=_type_setter) """Flag indicating the estimator type inferred from the input sources.""" + bids_id = attr.ib(default=None, kw_only=True, type=str, on_setattr=_id_setter) + """The unique ``B0FieldIdentifier`` field of this fieldmap.""" + def __attrs_post_init__(self): """Determine the inteded fieldmap estimation type and check for data completeness.""" suffix_list = [f.suffix for f in self.sources] @@ -349,3 +376,21 @@ def __attrs_post_init__(self): # No method has been identified -> fail. if self.method == EstimatorType.UNKNOWN: raise ValueError("Insufficient sources to estimate a fieldmap.") + + if not self.bids_id: + bids_ids = set([ + f.metadata.get("B0FieldIdentifier") + for f in self.sources if f.metadata.get("B0FieldIdentifier") + ]) + if len(bids_ids) > 1: + raise ValueError( + f"Multiple ``B0FieldIdentifier`` set: <{', '.join(bids_ids)}>" + ) + elif not bids_ids: + self.bids_id = True + else: + self.bids_id = bids_ids.pop() + elif self.bids_id in _unique_ids: + raise ValueError("Unique identifier has been previously registered.") + else: + _unique_ids.add(self.bids_id) diff --git a/sdcflows/tests/test_fieldmaps.py b/sdcflows/tests/test_fieldmaps.py index 9311d402b4..0ef85e016c 100644 --- a/sdcflows/tests/test_fieldmaps.py +++ b/sdcflows/tests/test_fieldmaps.py @@ -54,9 +54,28 @@ def test_FieldmapEstimation(testdata_dir, inputfiles, method, nsources): """Test errors.""" sub_dir = testdata_dir / "sub-01" - fe = FieldmapEstimation([sub_dir / f for f in inputfiles]) + sources = [sub_dir / f for f in inputfiles] + fe = FieldmapEstimation(sources) assert fe.method == method assert len(fe.sources) == nsources + assert fe.bids_id is not None and fe.bids_id.startswith("auto_") + + # Attempt to change bids_id + with pytest.raises(ValueError): + fe.bids_id = "other" + + # Setting the same value should not raise + fe.bids_id = fe.bids_id + + # Ensure duplicate B0FieldIdentifier are not accepted + with pytest.raises(ValueError): + FieldmapEstimation(sources, bids_id=fe.bids_id) + + # B0FieldIdentifier can be generated manually + # Creating two FieldmapEstimation objects from the same sources SHOULD fail + # or be better handled in the future (see #129). + fe2 = FieldmapEstimation(sources, bids_id=f"no{fe.bids_id}") + assert fe2.bids_id and fe2.bids_id.startswith("noauto_") @pytest.mark.parametrize( @@ -74,3 +93,38 @@ def test_FieldmapEstimationError(testdata_dir, inputfiles, errortype): with pytest.raises(errortype): FieldmapEstimation([sub_dir / f for f in inputfiles]) + + +def test_FieldmapEstimationIdentifier(testdata_dir): + """Check some use cases of B0FieldIdentifier.""" + with pytest.raises(ValueError): + FieldmapEstimation([ + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}), + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"}) + ]) # Inconsistent identifiers + + fe = FieldmapEstimation([ + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}), + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}) + ]) + assert fe.bids_id == "fmap_0" + + with pytest.raises(ValueError): + FieldmapEstimation([ + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}), + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}) + ]) # Consistent, but already exists + + fe = FieldmapEstimation([ + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"}), + FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz", + metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"}) + ]) + assert fe.bids_id == "fmap_1" diff --git a/sdcflows/workflows/base.py b/sdcflows/workflows/base.py index 4e61b58662..fe92e1c2e7 100644 --- a/sdcflows/workflows/base.py +++ b/sdcflows/workflows/base.py @@ -41,20 +41,20 @@ def init_fmap_preproc_wf( ... omp_nthreads=1, ... output_dir="/tmp", ... subject="1", - ... ) - [FieldmapEstimation(sources=<4 files>, method=), - FieldmapEstimation(sources=<4 files>, method=), - FieldmapEstimation(sources=<3 files>, method=), - FieldmapEstimation(sources=<2 files>, method=)] + ... ) # doctest: +ELLIPSIS + [FieldmapEstimation(sources=<4 files>, method=, bids_id='...'), + FieldmapEstimation(sources=<4 files>, method=, bids_id='...'), + FieldmapEstimation(sources=<3 files>, method=, bids_id='...'), + FieldmapEstimation(sources=<2 files>, method=, bids_id='...')] >>> init_fmap_preproc_wf( ... layout=layouts['testdata'], ... omp_nthreads=1, ... output_dir="/tmp", ... subject="HCP101006", - ... ) - [FieldmapEstimation(sources=<2 files>, method=), - FieldmapEstimation(sources=<2 files>, method=)] + ... ) # doctest: +ELLIPSIS + [FieldmapEstimation(sources=<2 files>, method=, bids_id='...'), + FieldmapEstimation(sources=<2 files>, method=, bids_id='...')] """ from ..fieldmaps import FieldmapEstimation, FieldmapFile