Skip to content

Commit 0b31473

Browse files
oestebanoesteban
committed
enh: keep a registry of already-used identifiers (and auto-generate new)
Resolves: #125.
1 parent c4ca825 commit 0b31473

File tree

3 files changed

+108
-9
lines changed

3 files changed

+108
-9
lines changed

‎sdcflows/fieldmaps.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
from niworkflows.utils.bids import relative_to_root
1010

1111

12+
_unique_ids = set()
13+
14+
1215
class MetadataError(ValueError):
1316
"""A better name for a specific value error."""
1417

@@ -59,6 +62,27 @@ def _type_setter(obj, attribute, value):
5962
return value
6063

6164

65+
def _id_setter(obj, attribute, value):
66+
"""Ensure uniqueness of B0FieldIdentifier metadata."""
67+
if obj.bids_id:
68+
if obj.bids_id != value:
69+
raise ValueError("Unique identifier is already set")
70+
_unique_ids.add(value)
71+
return value
72+
73+
if value is True:
74+
value = f"auto_{len([el for el in _unique_ids if el.startswith('auto_')])}"
75+
elif not value:
76+
raise ValueError("Invalid unique identifier")
77+
78+
if value in _unique_ids:
79+
raise ValueError("Unique identifier has been previously registered.")
80+
81+
_unique_ids.add(value)
82+
return value
83+
84+
85+
6286
@attr.s(slots=True)
6387
class FieldmapFile:
6488
"""
@@ -253,6 +277,9 @@ class FieldmapEstimation:
253277
method = attr.ib(init=False, default=EstimatorType.UNKNOWN, on_setattr=_type_setter)
254278
"""Flag indicating the estimator type inferred from the input sources."""
255279

280+
bids_id = attr.ib(default=None, kw_only=True, type=str, on_setattr=_id_setter)
281+
"""The unique ``B0FieldIdentifier`` field of this fieldmap."""
282+
256283
def __attrs_post_init__(self):
257284
"""Determine the inteded fieldmap estimation type and check for data completeness."""
258285
suffix_list = [f.suffix for f in self.sources]
@@ -349,3 +376,21 @@ def __attrs_post_init__(self):
349376
# No method has been identified -> fail.
350377
if self.method == EstimatorType.UNKNOWN:
351378
raise ValueError("Insufficient sources to estimate a fieldmap.")
379+
380+
if not self.bids_id:
381+
bids_ids = set([
382+
f.metadata.get("B0FieldIdentifier")
383+
for f in self.sources if f.metadata.get("B0FieldIdentifier")
384+
])
385+
if len(bids_ids) > 1:
386+
raise ValueError(
387+
f"Multiple ``B0FieldIdentifier`` set: <{', '.join(bids_ids)}>"
388+
)
389+
elif not bids_ids:
390+
self.bids_id = True
391+
else:
392+
self.bids_id = bids_ids.pop()
393+
elif self.bids_id in _unique_ids:
394+
raise ValueError("Unique identifier has been previously registered.")
395+
else:
396+
_unique_ids.add(self.bids_id)

‎sdcflows/tests/test_fieldmaps.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,28 @@ def test_FieldmapEstimation(testdata_dir, inputfiles, method, nsources):
5454
"""Test errors."""
5555
sub_dir = testdata_dir / "sub-01"
5656

57-
fe = FieldmapEstimation([sub_dir / f for f in inputfiles])
57+
sources = [sub_dir / f for f in inputfiles]
58+
fe = FieldmapEstimation(sources)
5859
assert fe.method == method
5960
assert len(fe.sources) == nsources
61+
assert fe.bids_id is not None and fe.bids_id.startswith("auto_")
62+
63+
# Attempt to change bids_id
64+
with pytest.raises(ValueError):
65+
fe.bids_id = "other"
66+
67+
# Setting the same value should not raise
68+
fe.bids_id = fe.bids_id
69+
70+
# Ensure duplicate B0FieldIdentifier are not accepted
71+
with pytest.raises(ValueError):
72+
FieldmapEstimation(sources, bids_id=fe.bids_id)
73+
74+
# B0FieldIdentifier can be generated manually
75+
# Creating two FieldmapEstimation objects from the same sources SHOULD fail
76+
# or be better handled in the future (see #129).
77+
fe2 = FieldmapEstimation(sources, bids_id=f"no{fe.bids_id}")
78+
assert fe2.bids_id and fe2.bids_id.startswith("noauto_")
6079

6180

6281
@pytest.mark.parametrize(
@@ -74,3 +93,38 @@ def test_FieldmapEstimationError(testdata_dir, inputfiles, errortype):
7493

7594
with pytest.raises(errortype):
7695
FieldmapEstimation([sub_dir / f for f in inputfiles])
96+
97+
98+
def test_FieldmapEstimationIdentifier(testdata_dir):
99+
"""Check some use cases of B0FieldIdentifier."""
100+
with pytest.raises(ValueError):
101+
FieldmapEstimation([
102+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz",
103+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}),
104+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz",
105+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"})
106+
]) # Inconsistent identifiers
107+
108+
fe = FieldmapEstimation([
109+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz",
110+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}),
111+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz",
112+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"})
113+
])
114+
assert fe.bids_id == "fmap_0"
115+
116+
with pytest.raises(ValueError):
117+
FieldmapEstimation([
118+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz",
119+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"}),
120+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz",
121+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_0"})
122+
]) # Consistent, but already exists
123+
124+
fe = FieldmapEstimation([
125+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_fieldmap.nii.gz",
126+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"}),
127+
FieldmapFile(testdata_dir / "sub-01" / "fmap/sub-01_magnitude.nii.gz",
128+
metadata={"Units": "Hz", "B0FieldIdentifier": "fmap_1"})
129+
])
130+
assert fe.bids_id == "fmap_1"

‎sdcflows/workflows/base.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,20 @@ def init_fmap_preproc_wf(
4141
... omp_nthreads=1,
4242
... output_dir="/tmp",
4343
... subject="1",
44-
... )
45-
[FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>),
46-
FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>),
47-
FieldmapEstimation(sources=<3 files>, method=<EstimatorType.PHASEDIFF: 3>),
48-
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>)]
44+
... ) # doctest: +ELLIPSIS
45+
[FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
46+
FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
47+
FieldmapEstimation(sources=<3 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
48+
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>, bids_id='...')]
4949
5050
>>> init_fmap_preproc_wf(
5151
... layout=layouts['testdata'],
5252
... omp_nthreads=1,
5353
... output_dir="/tmp",
5454
... subject="HCP101006",
55-
... )
56-
[FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PHASEDIFF: 3>),
57-
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>)]
55+
... ) # doctest: +ELLIPSIS
56+
[FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
57+
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>, bids_id='...')]
5858
5959
"""
6060
from ..fieldmaps import FieldmapEstimation, FieldmapFile

0 commit comments

Comments
 (0)