diff --git a/src/scippnexus/field.py b/src/scippnexus/field.py index b7e60721..183eb5d9 100644 --- a/src/scippnexus/field.py +++ b/src/scippnexus/field.py @@ -39,10 +39,14 @@ class DependsOn: value: str def absolute_path(self) -> str | None: - if self.value == '.': + if self.is_terminal: return None return posixpath.normpath(posixpath.join(self.parent, self.value)) + @property + def is_terminal(self) -> bool: + return self.value == '.' + def _is_time(obj): if (unit := obj.unit) is None: diff --git a/src/scippnexus/nxtransformations.py b/src/scippnexus/nxtransformations.py index dde03cc3..39c95f6b 100644 --- a/src/scippnexus/nxtransformations.py +++ b/src/scippnexus/nxtransformations.py @@ -310,18 +310,13 @@ def parse_depends_on_chain( file = parent.underlying.file visited = [depends_on.absolute_path()] try: - while depends_on.value != '.': + while not depends_on.is_terminal: transform, base = _locate_depends_on_target( file, depends_on, parent.definitions ) depends_on = DependsOn(parent=base, value=transform.attrs['depends_on']) chain.transformations[transform.name] = transform[()] - if depends_on.absolute_path() == visited[-1]: - depends_on.value = '.' - # Transform.from_object does not see the full chain, so it cannot - # detect this case. - chain.transformations[transform.name].depends_on = depends_on - elif depends_on.absolute_path() in visited: + if depends_on.absolute_path() in visited: raise ValueError( 'Circular depends_on chain detected: ' f'{[*visited, depends_on.absolute_path()]}' diff --git a/tests/nxtransformations_test.py b/tests/nxtransformations_test.py index b3611d0a..f48ba6d4 100644 --- a/tests/nxtransformations_test.py +++ b/tests/nxtransformations_test.py @@ -831,7 +831,7 @@ def test_compute_transformation_warns_if_transformation_missing_vector_attr( root[()] -def test_compute_positions_terminates_with_depends_on_to_self_absolute(h5root): +def test_compute_positions_raises_for_depends_on_self_absolute(h5root): instrument = snx.create_class(h5root, 'instrument', snx.NXinstrument) detector = create_detector(instrument) snx.create_field(detector, 'x_pixel_offset', sc.linspace('xx', -1, 1, 2, unit='m')) @@ -846,7 +846,6 @@ def test_compute_positions_terminates_with_depends_on_to_self_absolute(h5root): value = sc.scalar(6.5, unit='mm') offset = sc.spatial.translation(value=[1, 2, 3], unit='mm') vector = sc.vector(value=[0, 0, 1]) - t = value * vector value1 = snx.create_field(transformations, 't1', value) value1.attrs['depends_on'] = 't2' value1.attrs['transformation_type'] = 'translation' @@ -858,31 +857,12 @@ def test_compute_positions_terminates_with_depends_on_to_self_absolute(h5root): value2.attrs['transformation_type'] = 'translation' value2.attrs['vector'] = vector.value - t1 = sc.spatial.translations(dims=t.dims, values=t.values, unit=t.unit) * offset - t2 = sc.spatial.translations(dims=t.dims, values=t.values, unit=t.unit).to( - unit='cm' - ) root = make_group(h5root) - loaded = root[()] - result = snx.compute_positions(loaded) - origin = sc.vector([0, 0, 0], unit='m') - assert_identical( - result['instrument']['detector_0']['position'], - t2.to(unit='m') * t1.to(unit='m') * origin, - ) - assert_identical( - result['instrument']['detector_0']['data'].coords['position'], - t2.to(unit='m') - * t1.to(unit='m') - * sc.vectors( - dims=['xx', 'yy'], - values=[[[-1, -1, 0], [-1, 1, 0]], [[1, -1, 0], [1, 1, 0]]], - unit='m', - ), - ) + with pytest.raises(ValueError, match="Circular depends_on"): + _ = root[()] -def test_compute_positions_terminates_with_depends_on_to_self_relative(h5root): +def test_compute_positions_raises_for_depends_on_self_relative(h5root): instrument = snx.create_class(h5root, 'instrument', snx.NXinstrument) detector = create_detector(instrument) snx.create_field(detector, 'x_pixel_offset', sc.linspace('xx', -1, 1, 2, unit='m')) @@ -897,7 +877,6 @@ def test_compute_positions_terminates_with_depends_on_to_self_relative(h5root): value = sc.scalar(6.5, unit='mm') offset = sc.spatial.translation(value=[1, 2, 3], unit='mm') vector = sc.vector(value=[0, 0, 1]) - t = value * vector value1 = snx.create_field(transformations, 't1', value) value1.attrs['depends_on'] = 't2' value1.attrs['transformation_type'] = 'translation' @@ -909,28 +888,9 @@ def test_compute_positions_terminates_with_depends_on_to_self_relative(h5root): value2.attrs['transformation_type'] = 'translation' value2.attrs['vector'] = vector.value - t1 = sc.spatial.translations(dims=t.dims, values=t.values, unit=t.unit) * offset - t2 = sc.spatial.translations(dims=t.dims, values=t.values, unit=t.unit).to( - unit='cm' - ) root = make_group(h5root) - loaded = root[()] - result = snx.compute_positions(loaded) - origin = sc.vector([0, 0, 0], unit='m') - assert_identical( - result['instrument']['detector_0']['position'], - t2.to(unit='m') * t1.to(unit='m') * origin, - ) - assert_identical( - result['instrument']['detector_0']['data'].coords['position'], - t2.to(unit='m') - * t1.to(unit='m') - * sc.vectors( - dims=['xx', 'yy'], - values=[[[-1, -1, 0], [-1, 1, 0]], [[1, -1, 0], [1, 1, 0]]], - unit='m', - ), - ) + with pytest.raises(ValueError, match="Circular depends_on"): + _ = root[()] def test_compute_positions_raises_for_depends_on_cycle(h5root):