From 3a42584bbe5fcd6e1b838d450ed163696d69d7be Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 12 Feb 2024 18:55:42 +0100 Subject: [PATCH 01/15] add test of `PartSegCore.io_utils` --- package/PartSegCore/io_utils.py | 61 ++++++++++------------- package/tests/test_PartSegCore/test_io.py | 54 +++++++++++++++++++- 2 files changed, 77 insertions(+), 38 deletions(-) diff --git a/package/PartSegCore/io_utils.py b/package/PartSegCore/io_utils.py index f9870b93e..1a160ffa7 100644 --- a/package/PartSegCore/io_utils.py +++ b/package/PartSegCore/io_utils.py @@ -1,3 +1,4 @@ +import io import json import os import re @@ -42,7 +43,7 @@ def check_segmentation_type(tar_file: TarFile) -> SegmentationType: return SegmentationType.analysis if "metadata.json" in names: return SegmentationType.mask - raise WrongFileTypeException + raise WrongFileTypeException # pragma: no cover def get_tarinfo(name, buffer: typing.Union[BytesIO, StringIO]): @@ -56,7 +57,23 @@ def get_tarinfo(name, buffer: typing.Union[BytesIO, StringIO]): return tar_info -class SaveBase(AlgorithmDescribeBase, ABC): +class _IOBase(AlgorithmDescribeBase, ABC): + @classmethod + def get_name_with_suffix(cls): + return cls.get_name() + + @classmethod + def get_extensions(cls) -> typing.List[str]: + match = re.match(r".*\((.*)\)", cls.get_name()) + if match is None: + raise ValueError(f"No extensions found in {cls.get_name()}") + extensions = match[1].split(" ") + if not all(x.startswith("*.") for x in extensions): + raise ValueError(f"Error with parsing extensions in {cls.get_name()}") + return [x[1:] for x in extensions] + + +class SaveBase(_IOBase, ABC): need_functions: typing.ClassVar[typing.List[str]] = [ "save", "get_short_name", @@ -91,15 +108,6 @@ def save( """ raise NotImplementedError - @classmethod - def get_name_with_suffix(cls): - return cls.get_name() - - @classmethod - def get_default_extension(cls): - match = re.search(r"\(\*(\.\w+)", cls.get_name_with_suffix()) - return match[1] if match else "" - @classmethod def need_segmentation(cls): return True @@ -109,17 +117,12 @@ def need_mask(cls): return False @classmethod - def get_extensions(cls) -> typing.List[str]: - match = re.match(r".*\((.*)\)", cls.get_name()) - if match is None: - raise ValueError(f"No extensions found in {cls.get_name()}") - extensions = match[1].split(" ") - if not all(x.startswith("*.") for x in extensions): - raise ValueError(f"Error with parsing extensions in {cls.get_name()}") - return [x[1:] for x in extensions] + def get_default_extension(cls): + match = re.search(r"\(\*(\.\w+)", cls.get_name_with_suffix()) + return match[1] if match else "" -class LoadBase(AlgorithmDescribeBase, ABC): +class LoadBase(_IOBase, ABC): need_functions: typing.ClassVar[typing.List[str]] = [ "load", "get_short_name", @@ -155,20 +158,6 @@ def load( """ raise NotImplementedError - @classmethod - def get_name_with_suffix(cls): - return cls.get_name() - - @classmethod - def get_extensions(cls) -> typing.List[str]: - match = re.match(r".*\((.*)\)", cls.get_name()) - if match is None: - raise ValueError(f"No extensions found in {cls.get_name()}") - extensions = match[1].split(" ") - if not all(x.startswith("*.") for x in extensions): - raise ValueError(f"Error with parsing extensions in {cls.get_name()}") - return [x[1:] for x in extensions] - @classmethod def get_fields(cls): return [] @@ -192,9 +181,9 @@ def partial(cls): return False -def load_metadata_base(data: typing.Union[str, Path]): +def load_metadata_base(data: typing.Union[str, Path, typing.TextIO]): try: - if isinstance(data, typing.TextIO): + if isinstance(data, io.TextIOBase): decoded_data = json.load(data, object_hook=partseg_object_hook) elif os.path.exists(data): with open(data, encoding="utf-8") as ff: diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index b61be80b2..84739568d 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -29,6 +29,7 @@ LoadBase, LoadPlanExcel, LoadPlanJson, + LoadPoints, SaveBase, SaveROIAsNumpy, find_problematic_entries, @@ -303,7 +304,7 @@ def test_modernize_0_9_2_3(self, bundle_test_dir): data = load_metadata_base(file_path) def test_update_name(self): - data = load_metadata_base(update_name_json) + data = load_metadata_base(UPDATE_NAME_JSON) mp = data["problematic set"] assert isinstance(mp, MeasurementProfile) assert isinstance(mp.chosen_fields[0], MeasurementEntry) @@ -327,6 +328,13 @@ def test_load_workflow(self, bundle_test_dir): for entry in measurement_profile.chosen_fields: assert entry.calculation_tree.name in MEASUREMENT_DICT + def test_load_workflow_from_text(self, bundle_test_dir): + with open(os.path.join(bundle_test_dir, "workflow.json")) as ff: + data_text = ff.read() + assert isinstance(load_metadata_base(data_text)["workflow"], CalculationPlan) + with open(os.path.join(bundle_test_dir, "workflow.json")) as ff: + isinstance(load_metadata_base(ff)["workflow"], CalculationPlan) + class TestSegmentationMask: def test_load_seg(self, data_test_dir): @@ -643,6 +651,7 @@ def test_json_parameters_mask_2(stack_segmentation1, tmp_path): @pytest.mark.parametrize("file_path", (Path(__file__).parent.parent / "test_data" / "notebook").glob("*.json")) def test_load_notebook_json(file_path): + """Check if all notebook files can be loaded""" load_metadata_base(file_path) @@ -755,7 +764,39 @@ def test_load_image_for_batch(data_test_dir): assert proj.mask is None -update_name_json = """ +def test_save_base_extension_parse_no_ext(): + class Save(SaveBase): + @classmethod + def get_name(cls) -> str: + return "Sample save" + + with pytest.raises(ValueError, match="No extensions"): + Save.get_extensions() + + +def test_save_base_extension_parse_nmalformated_ext(): + class Save(SaveBase): + @classmethod + def get_name(cls) -> str: + return "Sample save (a.txt)" + + with pytest.raises(ValueError, match="Error with parsing"): + Save.get_extensions() + + +def test_load_points(tmp_path): + data_path = tmp_path / "sample.csv" + with data_path.open("w") as fp: + fp.write(POINTS_DATA) + + res = LoadPoints.load([data_path]) + assert res.file_path == data_path + assert res.points.shape == (5, 4) + + assert LoadPoints.get_short_name() == "point_csv" + + +UPDATE_NAME_JSON = """ {"problematic set": { "__MeasurementProfile__": true, "name": "problematic set", @@ -834,3 +875,12 @@ def test_load_image_for_batch(data_test_dir): } } """ + + +POINTS_DATA = """index,axis-0,axis-1,axis-2,axis-3 +0.0,0.0,22.0,227.90873370570543,65.07832834070409 +1.0,0.0,22.0,91.94021739981048,276.7482348060973 +2.0,0.0,22.0,194.83531082048773,380.3782931797794 +3.0,0.0,22.0,391.07095327277943,268.6636259303818 +4.0,0.0,22.0,152.20734354620717,256.1692217292996 +""" From 003b7bda8c5240d652afda9a29c1fc8b6535e8aa Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 12 Feb 2024 18:56:38 +0100 Subject: [PATCH 02/15] add test of `PartSegCore.algorithm_describe_base` --- .../PartSegCore/algorithm_describe_base.py | 14 --------- .../test_algorithm_describe_base.py | 29 +++++++++++++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/package/PartSegCore/algorithm_describe_base.py b/package/PartSegCore/algorithm_describe_base.py index d0589fed7..bf679e592 100644 --- a/package/PartSegCore/algorithm_describe_base.py +++ b/package/PartSegCore/algorithm_describe_base.py @@ -3,7 +3,6 @@ import typing import warnings from abc import ABC, ABCMeta, abstractmethod -from enum import Enum from functools import wraps from local_migrator import REGISTER, class_to_str @@ -535,19 +534,6 @@ def _pretty_print( res += "\n" return res[:-1] - @classmethod - def print_dict(cls, dkt, indent=0, name: str = "") -> str: - if isinstance(dkt, Enum): - return dkt.name - if not isinstance(dkt, typing.MutableMapping): - # FIXME update in future method of proper printing channel number - if name.startswith("channel") and isinstance(dkt, int): - return str(dkt + 1) - return str(dkt) - return "\n" + "\n".join( - " " * indent + f"{k.replace('_', ' ')}: {cls.print_dict(v, indent + 2, k)}" for k, v in dkt.items() - ) - def __eq__(self, other): return ( isinstance(other, self.__class__) diff --git a/package/tests/test_PartSegCore/test_algorithm_describe_base.py b/package/tests/test_PartSegCore/test_algorithm_describe_base.py index 408beb464..5333ced39 100644 --- a/package/tests/test_PartSegCore/test_algorithm_describe_base.py +++ b/package/tests/test_PartSegCore/test_algorithm_describe_base.py @@ -21,6 +21,28 @@ from PartSegImage import Channel +def test_algorithm_property(): + ap = AlgorithmProperty("test", "Test", 1) + assert ap.name == "test" + assert "user_name='Test'" in repr(ap) + + +def test_algorithm_property_warn(): + with pytest.warns(DeprecationWarning, match="use value_type instead"): + ap = AlgorithmProperty("test", "Test", 1, property_type=int) + assert ap.value_type == int + + +def test_algorithm_property_no_kwargs(): + with pytest.raises(ValueError, match="are not expected"): + AlgorithmProperty("test", "Test", 1, a=1) + + +def test_algorithm_property_list_exc(): + with pytest.raises(ValueError, match="should be one of possible values"): + AlgorithmProperty("test", "Test", 1, possible_values=[2, 3], value_type=list) + + def test_get_description_class(): class SampleClass: __test_class__ = _GetDescriptionClass() @@ -77,6 +99,11 @@ def get_fields(cls) -> typing.List[typing.Union[AlgorithmProperty, str]]: assert TestSelection["test1"] is Class1 + assert TestSelection.__register__ != TestSelection2.__register__ + + ts = TestSelection(name="test1", values={}) + assert ts.algorithm() == Class1 + def test_algorithm_selection_convert_subclass(clean_register): class TestSelection(AlgorithmSelection): @@ -364,6 +391,8 @@ def test_roi_extraction_profile(self): def test_pretty_print(self): prof1 = ROIExtractionProfile(name="aaa", algorithm="aaa", values={}) assert f"{prof1}\n " == prof1.pretty_print(AnalysisAlgorithmSelection) + prof1 = ROIExtractionProfile(name="", algorithm="aaa", values={}) + assert f"{prof1}\n " == prof1.pretty_print(AnalysisAlgorithmSelection) prof2 = ROIExtractionProfile( name="aaa", algorithm=LowerThresholdAlgorithm.get_name(), From ef6cfeae6dfe5d604bf2d31566411eb30380bbbb Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 12 Feb 2024 19:03:30 +0100 Subject: [PATCH 03/15] add test of `PartSegCore.mask.io_functions` --- package/tests/test_PartSegCore/test_io.py | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index 84739568d..6d496bddb 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -483,6 +483,32 @@ def test_load_project_with_history(self, tmp_path, stack_segmentation1, mask_pro cmp_dict = {str(k): v for k, v in stack_segmentation1.roi_extraction_parameters.items()} assert str(res.history[0].roi_extraction_parameters["parameters"]) == str(cmp_dict) + def test_mask_project_tuple(self): + mask = np.zeros((10, 10), dtype=np.uint8) + mask[1:-1, 1:-1] = 2 + mask2 = np.copy(mask) + mask2[2:-2, 2:-2] = 4 + roi_info = ROIInfo(mask2) + mask_prop = MaskProperty.simple_mask() + elem = HistoryElement.create(roi_info, mask, {}, mask_prop) + proj = MaskProjectTuple( + file_path="test_data.tiff", + image=Image(np.zeros((10, 10), dtype=np.uint8), (1, 1), "", axes_order="YX"), + mask=mask, + roi_info=roi_info, + history=[elem], + selected_components=[1, 2], + roi_extraction_parameters={}, + ) + assert not proj.is_raw() + assert proj.is_masked() + raw_proj = proj.get_raw_copy() + assert raw_proj.is_raw() + assert not raw_proj.is_masked() + raw_masked_proj = proj.get_raw_mask_copy() + assert raw_masked_proj.is_masked() + assert raw_masked_proj.is_raw() + class TestSaveFunctions: @staticmethod From 86c821374d4f99a2e38283942297ca50d257699b Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Mon, 12 Feb 2024 22:11:02 +0100 Subject: [PATCH 04/15] improve tests pointed by coderabbit --- .../test_PartSegCore/test_algorithm_describe_base.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/package/tests/test_PartSegCore/test_algorithm_describe_base.py b/package/tests/test_PartSegCore/test_algorithm_describe_base.py index 5333ced39..7f1f6861d 100644 --- a/package/tests/test_PartSegCore/test_algorithm_describe_base.py +++ b/package/tests/test_PartSegCore/test_algorithm_describe_base.py @@ -389,10 +389,11 @@ def test_roi_extraction_profile(self): ROIExtractionProfile("aaa", "aaa", {}) def test_pretty_print(self): - prof1 = ROIExtractionProfile(name="aaa", algorithm="aaa", values={}) - assert f"{prof1}\n " == prof1.pretty_print(AnalysisAlgorithmSelection) - prof1 = ROIExtractionProfile(name="", algorithm="aaa", values={}) - assert f"{prof1}\n " == prof1.pretty_print(AnalysisAlgorithmSelection) + + prof1 = ROIExtractionProfile(name="aaa", algorithm="Lower threshold", values={}) + assert prof1.pretty_print(AnalysisAlgorithmSelection).startswith("ROI extraction profile name:") + prof1 = ROIExtractionProfile(name="", algorithm="Lower threshold", values={}) + assert prof1.pretty_print(AnalysisAlgorithmSelection).startswith("ROI extraction profile\n") prof2 = ROIExtractionProfile( name="aaa", algorithm=LowerThresholdAlgorithm.get_name(), From 6690abc49c77add2f07ab983556201d2ec7935f9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Tue, 13 Feb 2024 19:10:21 +0100 Subject: [PATCH 05/15] test registering methods --- .../PartSegCore/algorithm_describe_base.py | 19 +++-- .../test_algorithm_describe_base.py | 72 +++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/package/PartSegCore/algorithm_describe_base.py b/package/PartSegCore/algorithm_describe_base.py index 94071b45a..5c859f8c0 100644 --- a/package/PartSegCore/algorithm_describe_base.py +++ b/package/PartSegCore/algorithm_describe_base.py @@ -210,6 +210,8 @@ def get_fields_from_algorithm(ald_desc: AlgorithmDescribeBase) -> typing.List[ty def is_static(fun): + if fun is None: + return False args = inspect.getfullargspec(fun).args return True if len(args) == 0 else args[0] != "self" @@ -255,6 +257,9 @@ def __eq__(self, other): and self.suggested_base_class == other.suggested_base_class ) + def __ne__(self, other): + return not self.__eq__(other) + def __getitem__(self, item) -> AlgorithmType: # FIXME add better strategy to get proper class when there is conflict of names try: @@ -277,11 +282,11 @@ def register( self.check_function(value, "get_name", True) try: name = value.get_name() - except NotImplementedError: - raise ValueError(f"Class {value} need to implement get_name class method") from None + except (NotImplementedError, AttributeError): + raise ValueError(f"Class {value} need to implement classmethod 'get_name'") from None if name in self and not replace: raise ValueError( - f"Object {self[name]} with this name: {name} already exist and register is not in replace mode" + f"Object {self[name]} with this name: '{name}' already exist and register is not in replace mode" ) if not isinstance(name, str): raise ValueError(f"Function get_name of class {value} need return string not {type(name)}") @@ -291,8 +296,8 @@ def register( for old_name in old_names: if old_name in self._old_mapping and not replace: raise ValueError( - f"Old value mapping for name {old_name} already registered." - f" Currently pointing to {self._old_mapping[name]}" + f"Old value mapping for name '{old_name}' already registered." + f" Currently pointing to {self._old_mapping[old_name]}" ) self._old_mapping[old_name] = name return value @@ -303,7 +308,7 @@ def check_function(ob, function_name, is_class): if not is_class and not inspect.isfunction(fun): raise ValueError(f"Class {ob} need to define method {function_name}") if is_class and not inspect.ismethod(fun) and not is_static(fun): - raise ValueError(f"Class {ob} need to define classmethod {function_name}") + raise ValueError(f"Class {ob} need to define classmethod '{function_name}'") def __setitem__(self, key: str, value: AlgorithmType): if not issubclass(value, AlgorithmDescribeBase): @@ -414,7 +419,7 @@ def register( :param replace: replace existing algorithm, be patient with :param old_names: list of old names for registered class """ - return cls.__register__.register(value, replace, old_names) + return cls.__register__.register(value, replace=replace, old_names=old_names) @classmethod def get_default(cls): diff --git a/package/tests/test_PartSegCore/test_algorithm_describe_base.py b/package/tests/test_PartSegCore/test_algorithm_describe_base.py index 7f1f6861d..e7230726b 100644 --- a/package/tests/test_PartSegCore/test_algorithm_describe_base.py +++ b/package/tests/test_PartSegCore/test_algorithm_describe_base.py @@ -105,6 +105,78 @@ def get_fields(cls) -> typing.List[typing.Union[AlgorithmProperty, str]]: assert ts.algorithm() == Class1 +def test_register_errors(): + class TestSelection(AlgorithmSelection): + pass + + class Alg1: + pass + + class Alg2(AlgorithmDescribeBase): + pass + + class Alg3(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return 1 + + @classmethod + def get_fields(cls): + return [] + + with pytest.raises(ValueError, match="Class .* need to define classmethod 'get_name'"): + TestSelection.register(Alg1) + + with pytest.raises(ValueError, match="Class .* need to implement classmethod 'get_name'"): + TestSelection.register(Alg2) + + with pytest.raises(ValueError, match="Function get_name of class .* need return string not .*int"): + TestSelection.register(Alg3) + + +def test_register_name_collsion(): + class TestSelection(AlgorithmSelection): + pass + + class Alg1(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return "1" + + @classmethod + def get_fields(cls): + return [] + + class Alg2(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return "1" + + @classmethod + def get_fields(cls): + return [] + + class Alg3(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return "2" + + @classmethod + def get_fields(cls): + return [] + + TestSelection.register(Alg1, old_names=["0"]) + with pytest.raises( + ValueError, match="Object .* with this name: '1' already exist and register is not in replace mode" + ): + TestSelection.register(Alg2) + + assert len(TestSelection.__register__) == 1 + + with pytest.raises(ValueError, match="Old value mapping for name '0' already registered"): + TestSelection.register(Alg3, old_names=["0"]) + + def test_algorithm_selection_convert_subclass(clean_register): class TestSelection(AlgorithmSelection): pass From 87e7711fb851229081b9148c6cb785bc33dcf259 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Tue, 13 Feb 2024 22:07:39 +0100 Subject: [PATCH 06/15] test agortithm describe base --- .../PartSegCore/algorithm_describe_base.py | 8 +- .../test_algorithm_describe_base.py | 81 ++++++++++++++++++- 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/package/PartSegCore/algorithm_describe_base.py b/package/PartSegCore/algorithm_describe_base.py index 5c859f8c0..c47a15cf0 100644 --- a/package/PartSegCore/algorithm_describe_base.py +++ b/package/PartSegCore/algorithm_describe_base.py @@ -313,18 +313,18 @@ def check_function(ob, function_name, is_class): def __setitem__(self, key: str, value: AlgorithmType): if not issubclass(value, AlgorithmDescribeBase): raise ValueError( - f"Class {value} need to inherit from {AlgorithmDescribeBase.__module__}.AlgorithmDescribeBase" + f"Class {value} need to be subclass of {AlgorithmDescribeBase.__module__}.AlgorithmDescribeBase" ) self.check_function(value, "get_name", True) self.check_function(value, "get_fields", True) try: val = value.get_name() - except NotImplementedError: - raise ValueError(f"Method get_name of class {value} need to be implemented") from None + except (NotImplementedError, AttributeError): + raise ValueError(f"Class {value} need to implement classmethod 'get_name'") from None if not isinstance(val, str): raise ValueError(f"Function get_name of class {value} need return string not {type(val)}") if key != val: - raise ValueError("Object need to be registered under name returned by gey_name function") + raise ValueError("Object need to be registered under name returned by get_name function") if not value.__new_style__: try: val = value.get_fields() diff --git a/package/tests/test_PartSegCore/test_algorithm_describe_base.py b/package/tests/test_PartSegCore/test_algorithm_describe_base.py index e7230726b..528690108 100644 --- a/package/tests/test_PartSegCore/test_algorithm_describe_base.py +++ b/package/tests/test_PartSegCore/test_algorithm_describe_base.py @@ -130,11 +130,14 @@ def get_fields(cls): with pytest.raises(ValueError, match="Class .* need to implement classmethod 'get_name'"): TestSelection.register(Alg2) + with pytest.raises(ValueError, match="Class .* need to implement classmethod 'get_name'"): + TestSelection.__register__["test1"] = Alg2 + with pytest.raises(ValueError, match="Function get_name of class .* need return string not .*int"): TestSelection.register(Alg3) -def test_register_name_collsion(): +def test_register_name_collision(): class TestSelection(AlgorithmSelection): pass @@ -154,7 +157,7 @@ def get_name(cls): @classmethod def get_fields(cls): - return [] + return [] # pragma: no cover class Alg3(AlgorithmDescribeBase): @classmethod @@ -177,6 +180,80 @@ def get_fields(cls): TestSelection.register(Alg3, old_names=["0"]) +def test_register_not_subclass(): + class TestSelection(AlgorithmSelection): + pass + + class Alg1: + @classmethod + def get_name(cls): + return "1" + + @classmethod + def get_fields(cls): + return [] + + with pytest.raises(ValueError, match="Class .* need to be subclass of .*AlgorithmDescribeBase"): + TestSelection.register(Alg1) + + +def test_register_validate_name_assignment(): + class TestSelection(AlgorithmSelection): + pass + + class Alg1(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return "1" + + @classmethod + def get_fields(cls): + return [] + + class Alg2(Alg1): + @classmethod + def get_name(cls): + return 2 + + with pytest.raises(ValueError, match="need return string"): + TestSelection.__register__["1"] = Alg2 + + with pytest.raises(ValueError, match="under name returned by get_name function"): + TestSelection.__register__["2"] = Alg1 + + +def test_register_get_fields_validity(): + class TestSelection(AlgorithmSelection): + pass + + class Alg1(AlgorithmDescribeBase): + @classmethod + def get_name(cls): + return "1" + + @classmethod + def get_fields(cls): + raise NotImplementedError + + class Alg2(Alg1): + @classmethod + def get_fields(cls): + return () + + with pytest.raises(ValueError, match="need to be implemented"): + TestSelection.register(Alg1) + with pytest.raises(ValueError, match="need return list not"): + TestSelection.register(Alg2) + + +def test_register_no_default_value(): + class TestSelection(AlgorithmSelection): + pass + + with pytest.raises(ValueError, match="Register does not contain any algorithm"): + TestSelection.get_default() + + def test_algorithm_selection_convert_subclass(clean_register): class TestSelection(AlgorithmSelection): pass From b318b7c34e1af54287a3fef9686cefe185805e6e Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 11:17:20 +0100 Subject: [PATCH 07/15] exclude lines intentionally not covered --- .../tests/test_PartSegCore/test_algorithm_describe_base.py | 6 +++--- pyproject.toml | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/package/tests/test_PartSegCore/test_algorithm_describe_base.py b/package/tests/test_PartSegCore/test_algorithm_describe_base.py index 528690108..bee4ad813 100644 --- a/package/tests/test_PartSegCore/test_algorithm_describe_base.py +++ b/package/tests/test_PartSegCore/test_algorithm_describe_base.py @@ -122,7 +122,7 @@ def get_name(cls): @classmethod def get_fields(cls): - return [] + return [] # pragma: no cover with pytest.raises(ValueError, match="Class .* need to define classmethod 'get_name'"): TestSelection.register(Alg1) @@ -191,7 +191,7 @@ def get_name(cls): @classmethod def get_fields(cls): - return [] + return [] # pragma: no cover with pytest.raises(ValueError, match="Class .* need to be subclass of .*AlgorithmDescribeBase"): TestSelection.register(Alg1) @@ -208,7 +208,7 @@ def get_name(cls): @classmethod def get_fields(cls): - return [] + return [] # pragma: no cover class Alg2(Alg1): @classmethod diff --git a/pyproject.toml b/pyproject.toml index 5793e653b..16e4ea945 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -221,6 +221,7 @@ omit = [ ".tox/*", "**/changelog.py", "**/version.py", + "package/PartSegCore/class_generator.py", ] parallel = true From 99d656f124c4954b39c43a4f835421bc5daa2c0c Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 11:45:18 +0100 Subject: [PATCH 08/15] more `PartSegCore.io_utils` tests --- package/PartSegCore/io_utils.py | 9 +-- package/tests/test_PartSegCore/test_io.py | 67 +++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/package/PartSegCore/io_utils.py b/package/PartSegCore/io_utils.py index 1a160ffa7..50c0d0223 100644 --- a/package/PartSegCore/io_utils.py +++ b/package/PartSegCore/io_utils.py @@ -288,7 +288,7 @@ def open_tar_file( tar_file = TarFile.open(fileobj=file_data) file_path = "" else: - raise ValueError(f"wrong type of file_ argument: {type(file_data)}") + raise ValueError(f"wrong type of file_data argument: {type(file_data)}") return tar_file, file_path @@ -314,13 +314,14 @@ def save( cls, save_location: typing.Union[str, BytesIO, Path], project_info, - parameters: dict, + parameters: typing.Optional[dict] = None, range_changed=None, step_changed=None, ): if project_info.image.mask is None and project_info.mask is not None: ImageWriter.save_mask(project_info.image.substitute(mask=project_info.mask), save_location) - ImageWriter.save_mask(project_info.image, save_location) + else: + ImageWriter.save_mask(project_info.image, save_location) def tar_to_buff(tar_file, member_name) -> BytesIO: @@ -341,7 +342,7 @@ def save( cls, save_location: typing.Union[str, BytesIO, Path], project_info, - parameters: dict, + parameters: typing.Optional[dict] = None, range_changed=None, step_changed=None, ): diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index 6d496bddb..ec6c70d3c 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -8,6 +8,7 @@ from copy import deepcopy from enum import Enum from glob import glob +from io import BytesIO from pathlib import Path from typing import Type @@ -31,11 +32,14 @@ LoadPlanJson, LoadPoints, SaveBase, + SaveMaskAsTiff, SaveROIAsNumpy, + SaveScreenshot, find_problematic_entries, find_problematic_leafs, load_metadata_base, load_metadata_part, + open_tar_file, ) from PartSegCore.json_hooks import PartSegEncoder, partseg_object_hook from PartSegCore.mask.history_utils import create_history_element_from_segmentation_tuple @@ -822,6 +826,69 @@ def test_load_points(tmp_path): assert LoadPoints.get_short_name() == "point_csv" +def test_open_tar_file_with_tarfile_object(tmp_path): + tar_file_path = tmp_path / "test.tar" + tar_file = tarfile.TarFile.open(tar_file_path, "w") + tar_file.close() + result_tar_file, result_file_path = open_tar_file(tar_file) + assert isinstance(result_tar_file, tarfile.TarFile) + assert result_file_path == "" + + +def test_open_tar_file_with_path(tmp_path): + tar_file_path = tmp_path / "test.tar" + tar_file = tarfile.TarFile.open(tar_file_path, "w") + tar_file.close() + result_tar_file, result_file_path = open_tar_file(tar_file_path) + assert isinstance(result_tar_file, tarfile.TarFile) + assert result_file_path == str(tar_file_path) + + +def test_open_tar_file_with_buffer(tmp_path): + buffer = BytesIO() + tar_file = tarfile.TarFile.open(fileobj=buffer, mode="w") + tar_file.close() + buffer.seek(0) + result_tar_file, result_file_path = open_tar_file(buffer) + assert isinstance(result_tar_file, tarfile.TarFile) + assert result_file_path == "" + + +def test_open_tar_file_with_invalid_type(tmp_path): + with pytest.raises(ValueError, match="wrong type of file_data argument"): + open_tar_file(123) + + +def test_save_mask_as_fiff(tmp_path, stack_segmentation1, analysis_segmentation2): + file_path = tmp_path / "test.tiff" + file_path2 = tmp_path / "test2.tiff" + SaveMaskAsTiff.save(file_path, stack_segmentation1) + assert not file_path.exists() + assert analysis_segmentation2.image.mask is None + SaveMaskAsTiff.save(file_path, analysis_segmentation2) + assert file_path.exists() + assert tifffile.TiffFile(str(file_path)).asarray().shape == analysis_segmentation2.mask.squeeze().shape + seg = dataclasses.replace( + analysis_segmentation2, image=analysis_segmentation2.image.substitute(mask=analysis_segmentation2.mask) + ) + SaveMaskAsTiff.save(file_path2, seg) + assert file_path2.exists() + assert tifffile.TiffFile(str(file_path2)).asarray().shape == analysis_segmentation2.mask.squeeze().shape + + +def test_save_screenshot(tmp_path): + file_path = tmp_path / "test.png" + data = np.zeros((100, 100, 3), dtype=np.uint8) + SaveScreenshot.save(file_path, data) + assert file_path.exists() + assert file_path.stat().st_size > 0 + + assert SaveScreenshot.get_default_extension() == ".png" + assert SaveScreenshot.get_short_name() == "screenshot" + assert SaveScreenshot.get_fields() == [] + assert {".png", ".jpg", ".jpeg"}.issubset(set(SaveScreenshot.get_extensions())) + + UPDATE_NAME_JSON = """ {"problematic set": { "__MeasurementProfile__": true, From a56b7f9845288b7aa48d672c82cc11dab4b9d126 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 12:11:49 +0100 Subject: [PATCH 09/15] add test for `PartSegCore.mask.io_functions` --- package/PartSegCore/mask/io_functions.py | 2 +- package/tests/test_PartSegCore/test_io.py | 47 +++++++++++++++++------ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/package/PartSegCore/mask/io_functions.py b/package/PartSegCore/mask/io_functions.py index 58ad92d28..2ec87fb53 100644 --- a/package/PartSegCore/mask/io_functions.py +++ b/package/PartSegCore/mask/io_functions.py @@ -106,7 +106,7 @@ def get_raw_mask_copy(self): return MaskProjectTuple(file_path=self.file_path, image=self.image.substitute(), mask=self.mask) @property - def roi(self): + def roi(self): # pragma: no cover warnings.warn("roi is deprecated", DeprecationWarning, stacklevel=2) return self.roi_info.roi diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index ec6c70d3c..3ab84675d 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -45,6 +45,7 @@ from PartSegCore.mask.history_utils import create_history_element_from_segmentation_tuple from PartSegCore.mask.io_functions import ( LoadROI, + LoadROIFromTIFF, LoadROIImage, LoadROIParameters, LoadStackImage, @@ -205,6 +206,10 @@ def test_save_roi_info_mask_project(self, stack_segmentation2, tmp_path): self.perform_roi_info_test(stack_segmentation2, tmp_path, SaveROI, LoadROI) def perform_roi_info_test(self, project, save_path, save_method: Type[SaveBase], load_method: Type[LoadBase]): + assert save_method.get_short_name().lower() == save_method.get_short_name() + assert save_method.get_short_name().isalpha() + assert load_method.get_short_name().lower() == load_method.get_short_name() + assert load_method.get_short_name().isalpha() alt1 = np.copy(project.roi_info.roi) alt1[alt1 > 0] += 3 roi_info = ROIInfo( @@ -876,17 +881,37 @@ def test_save_mask_as_fiff(tmp_path, stack_segmentation1, analysis_segmentation2 assert tifffile.TiffFile(str(file_path2)).asarray().shape == analysis_segmentation2.mask.squeeze().shape -def test_save_screenshot(tmp_path): - file_path = tmp_path / "test.png" - data = np.zeros((100, 100, 3), dtype=np.uint8) - SaveScreenshot.save(file_path, data) - assert file_path.exists() - assert file_path.stat().st_size > 0 - - assert SaveScreenshot.get_default_extension() == ".png" - assert SaveScreenshot.get_short_name() == "screenshot" - assert SaveScreenshot.get_fields() == [] - assert {".png", ".jpg", ".jpeg"}.issubset(set(SaveScreenshot.get_extensions())) +class TestSaveScreenshot: + def test_get_name(self): + assert SaveScreenshot.get_default_extension() == ".png" + assert SaveScreenshot.get_name().startswith("Screenshot") + assert SaveScreenshot.get_short_name() == "screenshot" + assert SaveScreenshot.get_fields() == [] + assert {".png", ".jpg", ".jpeg"}.issubset(set(SaveScreenshot.get_extensions())) + + def test_saving(self, tmp_path): + file_path = tmp_path / "test.png" + data = np.zeros((100, 100, 3), dtype=np.uint8) + SaveScreenshot.save(file_path, data) + assert file_path.exists() + assert file_path.stat().st_size > 0 + + +class TestLoadROIFromTIFF: + def test_get_name(self): + assert LoadROIFromTIFF.get_name().startswith("ROI from tiff") + assert LoadROIFromTIFF.get_short_name() == "roi_tiff" + assert set(LoadROIFromTIFF.get_extensions()) == {".tiff", ".tif"} + + def test_load(self, tmp_path): + data = np.zeros((100, 100), dtype=np.uint8) + data[10:20, 10:20] = 1 + data[30:40, 30:40] = 2 + tifffile.imwrite(tmp_path / "test.tiff", data=data) + res = LoadROIFromTIFF.load([tmp_path / "test.tiff"]) + assert isinstance(res, MaskProjectTuple) + assert res.image is None + assert res.roi_info.roi.shape == (1, 1, 100, 100) UPDATE_NAME_JSON = """ From 44158402fba426bd30bb3d509c24b121b6121209 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 12:15:37 +0100 Subject: [PATCH 10/15] Update package/tests/test_PartSegCore/test_io.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- package/tests/test_PartSegCore/test_io.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index 3ab84675d..70f68915e 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -809,7 +809,7 @@ def get_name(cls) -> str: Save.get_extensions() -def test_save_base_extension_parse_nmalformated_ext(): +def test_save_base_extension_parse_malformatted_ext(): class Save(SaveBase): @classmethod def get_name(cls) -> str: From 5f720d8d56e2e4af8ee71a239f77c121cd98c070 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 12:17:51 +0100 Subject: [PATCH 11/15] simplify test and set name --- package/tests/test_PartSegCore/test_io.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/package/tests/test_PartSegCore/test_io.py b/package/tests/test_PartSegCore/test_io.py index 70f68915e..bdcddff08 100644 --- a/package/tests/test_PartSegCore/test_io.py +++ b/package/tests/test_PartSegCore/test_io.py @@ -864,11 +864,9 @@ def test_open_tar_file_with_invalid_type(tmp_path): open_tar_file(123) -def test_save_mask_as_fiff(tmp_path, stack_segmentation1, analysis_segmentation2): +def test_save_mask_as_tiff(tmp_path, analysis_segmentation2): file_path = tmp_path / "test.tiff" file_path2 = tmp_path / "test2.tiff" - SaveMaskAsTiff.save(file_path, stack_segmentation1) - assert not file_path.exists() assert analysis_segmentation2.image.mask is None SaveMaskAsTiff.save(file_path, analysis_segmentation2) assert file_path.exists() From 8f9bbde8baf1e143b7530bb39e16c0c91cad57cb Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 12:48:30 +0100 Subject: [PATCH 12/15] try ommit class generator --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 16e4ea945..5c6426756 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -221,7 +221,7 @@ omit = [ ".tox/*", "**/changelog.py", "**/version.py", - "package/PartSegCore/class_generator.py", + "PartSegCore/class_generator.py", ] parallel = true From c64aa2f422f209ef8f164a7a007d61ede22921a9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 13:47:24 +0100 Subject: [PATCH 13/15] another approach --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 5c6426756..1d825f27a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -221,7 +221,7 @@ omit = [ ".tox/*", "**/changelog.py", "**/version.py", - "PartSegCore/class_generator.py", + "**/PartSegCore/class_generator.py", ] parallel = true From 787c55606b1ab63b34adef270dd2f16d36f3bd82 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 15:34:30 +0100 Subject: [PATCH 14/15] remove obsolete functions --- package/PartSegCore/analysis/load_functions.py | 5 ----- package/PartSegCore/io_utils.py | 9 ++------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/package/PartSegCore/analysis/load_functions.py b/package/PartSegCore/analysis/load_functions.py index 8b0b43b93..5b918ca18 100644 --- a/package/PartSegCore/analysis/load_functions.py +++ b/package/PartSegCore/analysis/load_functions.py @@ -221,11 +221,6 @@ def get_short_name(cls): def number_of_files(cls): return 2 - @classmethod - def correct_files_order(cls, paths): - name1, name2 = (os.path.basename(os.path.splitext(x)[0]) for x in paths) - return [name1, name2] if name2.endswith("_mask") else paths - @classmethod def load( cls, diff --git a/package/PartSegCore/io_utils.py b/package/PartSegCore/io_utils.py index 50c0d0223..2d4066df7 100644 --- a/package/PartSegCore/io_utils.py +++ b/package/PartSegCore/io_utils.py @@ -128,7 +128,6 @@ class LoadBase(_IOBase, ABC): "get_short_name", "get_name_with_suffix", "number_of_files", - "correct_files_order", "get_next_file", "partial", ] @@ -167,10 +166,6 @@ def number_of_files(cls): """Number of files required for load method""" return 1 - @classmethod - def correct_files_order(cls, paths): - return paths - @classmethod def get_next_file(cls, file_paths: typing.List[str]): return file_paths[0] @@ -190,10 +185,10 @@ def load_metadata_base(data: typing.Union[str, Path, typing.TextIO]): decoded_data = json.load(ff, object_hook=partseg_object_hook) else: decoded_data = json.loads(data, object_hook=partseg_object_hook) - except ValueError as e: + except ValueError as e: # pragma: no cover try: decoded_data = json.loads(str(data), object_hook=partseg_object_hook) - except Exception: # pragma: no cover + except Exception: raise e # noqa: B904 return decoded_data From 0701e71db26df4518e5bd836e9e16dcb1a505093 Mon Sep 17 00:00:00 2001 From: Grzegorz Bokota Date: Wed, 14 Feb 2024 15:49:09 +0100 Subject: [PATCH 15/15] add minor tests --- package/PartSegCore/autofit.py | 6 ++++-- package/PartSegCore/napari_plugins/loader.py | 4 ++-- .../tests/test_PartSegCore/test_napari_plugins.py | 13 +++++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/package/PartSegCore/autofit.py b/package/PartSegCore/autofit.py index b5db4e47a..c49d6f8dd 100644 --- a/package/PartSegCore/autofit.py +++ b/package/PartSegCore/autofit.py @@ -1,3 +1,4 @@ +import warnings from math import acos, pi, sqrt import numpy as np @@ -41,7 +42,7 @@ def find_density_orientation(img, voxel_size, cutoff=1): return vectors, w_n -def get_rotation_parameters(isometric_matrix): +def get_rotation_parameters(isometric_matrix): # pragma: no cover """ If 3x3 isometric matrix is not rotation matrix function transform it into rotation matrix @@ -49,6 +50,7 @@ def get_rotation_parameters(isometric_matrix): :param isometric_matrix: 3x3 np.ndarray with determinant equal 1 or -1 :return: rotation_matrix, rotation axis, rotation angel """ + warnings.warn("This function is deprecated", FutureWarning, stacklevel=2) if np.linalg.det(isometric_matrix) < 0: isometric_matrix = np.dot(np.diag([-1, 1, 1]), isometric_matrix) angel = acos((np.sum(np.diag(isometric_matrix)) - 1) / 2) * 180 / pi @@ -78,7 +80,7 @@ def density_mass_center(image, voxel_size=(1.0, 1.0, 1.0)): if len(voxel_size) != image.ndim: if len(voxel_size) != len(iter_dim): - raise ValueError("Cannot fit voxel size to array") + raise ValueError("Cannot fit voxel size to array") # pragma: no cover voxel_size_array = [0] * image.ndim for i, item in enumerate(iter_dim): voxel_size_array[item] = voxel_size[i] diff --git a/package/PartSegCore/napari_plugins/loader.py b/package/PartSegCore/napari_plugins/loader.py index 0af87a324..0d96e7f7e 100644 --- a/package/PartSegCore/napari_plugins/loader.py +++ b/package/PartSegCore/napari_plugins/loader.py @@ -95,9 +95,9 @@ def partseg_loader(loader: typing.Type[LoadBase], path: str): try: project_info = loader.load(load_locations) - except WrongFileTypeException: + except WrongFileTypeException: # pragma: no cover return None if isinstance(project_info, (ProjectTuple, MaskProjectTuple)): return project_to_layers(project_info) - return None + return None # pragma: no cover diff --git a/package/tests/test_PartSegCore/test_napari_plugins.py b/package/tests/test_PartSegCore/test_napari_plugins.py index 5ade78169..206de59e9 100644 --- a/package/tests/test_PartSegCore/test_napari_plugins.py +++ b/package/tests/test_PartSegCore/test_napari_plugins.py @@ -6,6 +6,7 @@ import pytest from napari.layers import Image, Labels, Layer +from PartSegCore.analysis import ProjectTuple from PartSegCore.mask.io_functions import LoadROIFromTIFF from PartSegCore.napari_plugins.load_image import napari_get_reader as napari_get_reader_image from PartSegCore.napari_plugins.load_mask_project import napari_get_reader as napari_get_reader_mask @@ -20,6 +21,7 @@ napari_write_labels as napari_write_labels_tiff, ) from PartSegImage import GenericImageReader +from PartSegImage import Image as PImage def test_project_to_layers_analysis(analysis_segmentation): @@ -41,6 +43,16 @@ def test_project_to_layers_analysis(analysis_segmentation): assert not l3.visible +def test_project_to_layers_roi(): + data = np.zeros((1, 1, 10, 10, 10), dtype=np.uint8) + img = PImage(data, image_spacing=(1, 1, 1), name="ROI") + proj = ProjectTuple(file_path="", image=img) + res = project_to_layers(proj) + assert len(res) == 1 + assert isinstance(res[0][0], np.ndarray) + assert res[0][2] == "labels" + + def test_project_to_layers_mask(stack_segmentation1): res = project_to_layers(stack_segmentation1) assert len(res) == 2 @@ -108,6 +120,7 @@ def test_save_load_axis_order(tmp_path): assert napari_write_labels_tiff(data_path, *layer.as_layer_data_tuple()[:2]) proj = LoadROIFromTIFF.load([data_path]) assert proj.roi_info.roi.shape == data.shape + assert napari_write_labels_tiff(str(tmp_path / "test.seg"), *layer.as_layer_data_tuple()[:2]) is None @pytest.fixture(params=[(1, 4), (1, 3), (3, 4), (3, 3), (10, 4)])