Skip to content

Commit

Permalink
enh: keep a registry of already-used identifiers (and auto-generate new)
Browse files Browse the repository at this point in the history
Resolves: #125.
  • Loading branch information
oesteban committed Nov 27, 2020
1 parent 5588e11 commit 5fee784
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 9 deletions.
45 changes: 45 additions & 0 deletions sdcflows/fieldmaps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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:
"""
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
56 changes: 55 additions & 1 deletion sdcflows/tests/test_fieldmaps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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"
16 changes: 8 additions & 8 deletions sdcflows/workflows/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,20 @@ def init_fmap_preproc_wf(
... omp_nthreads=1,
... output_dir="/tmp",
... subject="1",
... )
[FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>),
FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>),
FieldmapEstimation(sources=<3 files>, method=<EstimatorType.PHASEDIFF: 3>),
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>)]
... ) # doctest: +ELLIPSIS
[FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
FieldmapEstimation(sources=<4 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
FieldmapEstimation(sources=<3 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>, bids_id='...')]
>>> init_fmap_preproc_wf(
... layout=layouts['testdata'],
... omp_nthreads=1,
... output_dir="/tmp",
... subject="HCP101006",
... )
[FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PHASEDIFF: 3>),
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>)]
... ) # doctest: +ELLIPSIS
[FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PHASEDIFF: 3>, bids_id='...'),
FieldmapEstimation(sources=<2 files>, method=<EstimatorType.PEPOLAR: 2>, bids_id='...')]
"""
from ..fieldmaps import FieldmapEstimation, FieldmapFile
Expand Down

0 comments on commit 5fee784

Please sign in to comment.