Skip to content

Commit 1f4483a

Browse files
committed
Pr comments first batch
1 parent 4d3fbe3 commit 1f4483a

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

src/easyimaging/measurement/regions.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,9 @@ def y_end(self, value: sc.Variable):
197197
self._y_end = value
198198

199199
def _check_input_sequence(self, value: Sequence, name: str, typename: str, expected_type: any) -> None:
200+
"""
201+
Check if the input value is a sequence of the expected type and length.
202+
"""
200203
if not isinstance(value, Sequence) or len(value) != 2:
201204
raise TypeError(f'{name} must be a tuple or a list of two {typename}, got a {type(value).__name__}.')
202205
if not (isinstance(value[0], expected_type) and isinstance(value[1], expected_type)):
@@ -206,12 +209,18 @@ def _check_input_sequence(self, value: Sequence, name: str, typename: str, expec
206209
)
207210

208211
def _check_index(self, value: int, name: str) -> None:
212+
"""
213+
Check if the input value is a valid index (non-negative integer).
214+
"""
209215
if not isinstance(value, int):
210216
raise TypeError(f'{name} indice must be an integer.')
211217
if value < 0:
212218
raise ValueError(f'{name} indice must be non-negative.')
213219

214220
def _check_scalar(self, value: sc.Variable, name: str) -> None:
221+
"""
222+
Check if the input value is a scipp scalar with a unit of length.
223+
"""
215224
if value.dims:
216225
raise ValueError(f'{name} must be a scipp scalar (0-dimensional Variable).')
217226
try:
@@ -221,6 +230,9 @@ def _check_scalar(self, value: sc.Variable, name: str) -> None:
221230
raise UnitError(f"{name} must be a scipp scalar with a unit of length (e.g., 'm').") from e
222231

223232
def _single_coord_setter_check(self, value: sc.Variable, name: str) -> None:
233+
"""
234+
Check if the input value is a scipp scalar and if physical coordinates are set.
235+
"""
224236
if not isinstance(value, sc.Variable):
225237
raise TypeError(f'{name} must be a scipp scalar.')
226238
if not self._has_physical_coords:

tests/unit_tests/measurement/test_measurement.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from copy import copy
12
from pathlib import Path
23
from typing import MutableSequence
34

@@ -466,7 +467,7 @@ def test_positions_setter_invalid(self, valid_data_array_no_xy_coords, coordinat
466467
with pytest.raises(ValueError, match=f'Cannot set {coordinate} before setting all physical coordinate positions.'):
467468
setattr(measurement, coordinate, sc.arange('x', 0, 10, 1, unit='m'))
468469

469-
# Just a single test, other test-cases is covered by from_tiff_stack tests as both uses _validate_provided_coord()
470+
# Just a single test, other test-cases are covered by from_tiff_stack tests as both use _validate_provided_coord()
470471
@pytest.mark.parametrize('coordinate', ['x_positions', 'y_positions'], ids=['x_coordinate', 'y_coordinate'])
471472
def test_positions_setter_invalid_coordinate(self, valid_data_array, coordinate):
472473
# When
@@ -489,7 +490,7 @@ def test_set_physical_coord_positions_valid(self, valid_data_array_no_xy_coords)
489490
assert sc.identical(measurement._data_array.coords['y'], new_y_positions)
490491
assert measurement._has_physical_coords
491492

492-
# Just a single test for each, other test-cases is covered by from_tiff_stack tests as both uses _validate_provided_coord()
493+
# Just a single test for each, other test-cases are covered by from_tiff_stack tests as both use _validate_provided_coord()
493494
@pytest.mark.parametrize(
494495
'coordinates, coordinate_wrong',
495496
[
@@ -587,12 +588,15 @@ def test_regions_of_interest(self, valid_data_array, valid_roi):
587588
def test_regions_of_interest_removal_by_index(self, valid_data_array, valid_roi):
588589
# When
589590
measurement = Measurement(data_array=valid_data_array)
591+
extra_roi = copy(valid_roi)
590592
measurement.regions_of_interest.append(valid_roi)
593+
measurement.regions_of_interest.append(extra_roi) # Add a second ROI to ensure only the correct one is removed
591594
# Then
592595
del measurement.regions_of_interest[0]
593596
# Expect
594-
assert len(measurement.regions_of_interest) == 0
597+
assert len(measurement.regions_of_interest) == 1
595598
assert valid_roi not in measurement.regions_of_interest
599+
assert extra_roi in measurement.regions_of_interest
596600

597601
def test_regions_of_interest_removal_by_name(self, valid_data_array, valid_roi):
598602
# When

0 commit comments

Comments
 (0)