From 4c3b2afda77ff8a5959b442bda389f68a42f3523 Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Thu, 13 Jan 2022 12:58:39 +0000 Subject: [PATCH 1/7] Remove duplicate None check --- ome_zarr/writer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index 9c36603a..f815f3c2 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -104,8 +104,7 @@ def _validate_plate_acquisitions( "starttime", "endtime", ] - if acquisitions is None: - return + for acquisition in acquisitions: if not isinstance(acquisition, dict): raise ValueError(f"{acquisition} must be a dictionary") From 347da426df98db2a9653d9bdbf17e6b8bbc00377 Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Thu, 13 Jan 2022 13:05:16 +0000 Subject: [PATCH 2/7] Do not mutate input argument and returned validated copies --- ome_zarr/writer.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index f815f3c2..938a556c 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -70,15 +70,16 @@ def _validate_axes(axes: List[str], fmt: Format = CurrentFormat()) -> None: raise ValueError("5D data must have axes ('t', 'c', 'z', 'y', 'x')") -def _validate_well_images(images: List, fmt: Format = CurrentFormat()) -> None: +def _validate_well_images(images: List, fmt: Format = CurrentFormat()) -> List[dict]: VALID_KEYS = [ "acquisition", "path", ] - for index, image in enumerate(images): + validated_images = [] + for image in images: if isinstance(image, str): - images[index] = {"path": str(image)} + validated_images.append({"path": str(image)}) elif isinstance(image, dict): if any(e not in VALID_KEYS for e in image.keys()): LOGGER.debug("f{image} contains unspecified keys") @@ -88,13 +89,15 @@ def _validate_well_images(images: List, fmt: Format = CurrentFormat()) -> None: raise ValueError(f"{image} path must be of string type") if "acquisition" in image and not isinstance(image["acquisition"], int): raise ValueError(f"{image} acquisition must be of int type") + validated_images.append(image) else: raise ValueError(f"Unrecognized type for {image}") + return validated_images def _validate_plate_acquisitions( acquisitions: List[Dict], fmt: Format = CurrentFormat() -) -> None: +) -> List[Dict]: VALID_KEYS = [ "id", @@ -114,6 +117,7 @@ def _validate_plate_acquisitions( raise ValueError(f"{acquisition} must contain an id key") if not isinstance(acquisition["id"], int): raise ValueError(f"{acquisition} id must be of int type") + return acquisitions def write_multiscale( @@ -238,8 +242,7 @@ def write_plate_metadata( if field_count is not None: plate["field_count"] = field_count if acquisitions is not None: - _validate_plate_acquisitions(acquisitions) - plate["acquisitions"] = acquisitions + plate["acquisitions"] = _validate_plate_acquisitions(acquisitions) group.attrs["plate"] = plate @@ -264,9 +267,8 @@ def write_well_metadata( Defaults to the most current. """ - _validate_well_images(images) well = { - "images": images, + "images": _validate_well_images(images), "version": fmt.version, } group.attrs["well"] = well From df472344078d41a98254cbd3473ef2788fca7270 Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Thu, 13 Jan 2022 13:14:26 +0000 Subject: [PATCH 3/7] Add support for wells as list of strings or list of dictionaries --- ome_zarr/writer.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index 938a556c..4e567ab7 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -120,6 +120,30 @@ def _validate_plate_acquisitions( return acquisitions +def _validate_plate_wells(wells: List, fmt: Format = CurrentFormat()) -> List[dict]: + + VALID_KEYS = [ + "path", + ] + validated_wells = [] + if wells is None: + raise ValueError("Empty wells list") + for well in wells: + if isinstance(well, str): + validated_wells.append({"path": str(well)}) + elif isinstance(well, dict): + if any(e not in VALID_KEYS for e in well.keys()): + LOGGER.debug("f{well} contains unspecified keys") + if "path" not in well: + raise ValueError(f"{well} must contain an path key") + if not isinstance(well["path"], str): + raise ValueError(f"{well} path must be of str type") + validated_wells.append({"path": str(well)}) + else: + raise ValueError(f"Unrecognized type for {well}") + return validated_wells + + def write_multiscale( pyramid: List, group: zarr.Group, @@ -201,7 +225,7 @@ def write_plate_metadata( group: zarr.Group, rows: List[str], columns: List[str], - wells: List[str], + wells: Union[List[str], List[dict]], fmt: Format = CurrentFormat(), acquisitions: List[dict] = None, field_count: int = None, @@ -218,7 +242,7 @@ def write_plate_metadata( The list of names for the plate rows columns: list of str The list of names for the plate columns - wells: list of str + wells: list of str or list of dict The list of paths for the well groups fmt: Format The format of the ome_zarr data which should be used. @@ -234,7 +258,7 @@ def write_plate_metadata( plate: Dict[str, Union[str, int, List[Dict]]] = { "columns": [{"name": str(c)} for c in columns], "rows": [{"name": str(r)} for r in rows], - "wells": [{"path": str(wp)} for wp in wells], + "wells": _validate_plate_wells(wells), "version": fmt.version, } if name is not None: From 86183c2177d8a515dde0f1371a3c84fb2a5e1afc Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Thu, 13 Jan 2022 13:21:19 +0000 Subject: [PATCH 4/7] Add unit tests for wells passed as a list of dictionaries --- ome_zarr/writer.py | 4 ++-- tests/test_writer.py | 46 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index 4e567ab7..fecfccb1 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -126,7 +126,7 @@ def _validate_plate_wells(wells: List, fmt: Format = CurrentFormat()) -> List[di "path", ] validated_wells = [] - if wells is None: + if wells is None or len(wells) == 0: raise ValueError("Empty wells list") for well in wells: if isinstance(well, str): @@ -138,7 +138,7 @@ def _validate_plate_wells(wells: List, fmt: Format = CurrentFormat()) -> List[di raise ValueError(f"{well} must contain an path key") if not isinstance(well["path"], str): raise ValueError(f"{well} path must be of str type") - validated_wells.append({"path": str(well)}) + validated_wells.append(well) else: raise ValueError(f"Unrecognized type for {well}") return validated_wells diff --git a/tests/test_writer.py b/tests/test_writer.py index 2f0897ef..e0f01d56 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -343,14 +343,54 @@ def test_acquisitions_maximal(self): ( [0, 1], [{"name": "0"}, {"name": "1"}], - [{"id": 0, "invalid_key": "0"}], [{"id": "0"}, {"id": "1"}], ), ) - def test_unspecified_acquisition_keys(self, acquisitions): - a = [{"id": 0, "invalid_key": "0"}] + def test_invalid_acquisition_keys(self, acquisitions): + with pytest.raises(ValueError): + write_plate_metadata( + self.root, ["A"], ["1"], ["A/1"], acquisitions=acquisitions + ) + + def test_unspecified_acquisition_keys(self): + a = [{"id": 0, "unspecified_key": "0"}] write_plate_metadata(self.root, ["A"], ["1"], ["A/1"], acquisitions=a) assert "plate" in self.root.attrs + assert self.root.attrs["plate"]["acquisitions"] == a + + @pytest.mark.parametrize( + "wells", + (None, [], [1]), + ) + def test_invalid_well_list(self, wells): + with pytest.raises(ValueError): + write_plate_metadata(self.root, ["A"], ["1"], wells) + + @pytest.mark.parametrize( + "wells", + ( + [{"path": 0}], + [{"id": "test"}], + [{"path": "A/1"}, {"path": None}], + ), + ) + def test_invalid_well_keys(self, wells): + wells = [{"path": 0}] + with pytest.raises(ValueError): + write_plate_metadata(self.root, ["A"], ["1"], wells) + + def test_unspecified_well_keys(self): + wells = [ + {"path": "A/1", "unspecified_key": "alpha"}, + {"path": "A/2", "unspecified_key": "beta"}, + {"path": "B/1", "unspecified_key": "gamma"}, + ] + write_plate_metadata(self.root, ["A", "B"], ["1", "2"], wells) + assert "plate" in self.root.attrs + assert self.root.attrs["plate"]["columns"] == [{"name": "1"}, {"name": "2"}] + assert self.root.attrs["plate"]["rows"] == [{"name": "A"}, {"name": "B"}] + assert self.root.attrs["plate"]["version"] == CurrentFormat().version + assert self.root.attrs["plate"]["wells"] == wells class TestWellMetadata: From 01fc3c9ce1e965321defdd5a0a3e3da69b992fba Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Thu, 13 Jan 2022 13:49:06 +0000 Subject: [PATCH 5/7] Fix test for invalid well keys --- tests/test_writer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_writer.py b/tests/test_writer.py index e0f01d56..d4866248 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -375,7 +375,6 @@ def test_invalid_well_list(self, wells): ), ) def test_invalid_well_keys(self, wells): - wells = [{"path": 0}] with pytest.raises(ValueError): write_plate_metadata(self.root, ["A"], ["1"], wells) From 0ed8b0bc97958f2b3c36284ca76f379bbbd9013e Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Fri, 14 Jan 2022 08:44:00 +0000 Subject: [PATCH 6/7] Consistently use List[Union[str, dict]] --- ome_zarr/writer.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index fecfccb1..87f1ea58 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -70,7 +70,9 @@ def _validate_axes(axes: List[str], fmt: Format = CurrentFormat()) -> None: raise ValueError("5D data must have axes ('t', 'c', 'z', 'y', 'x')") -def _validate_well_images(images: List, fmt: Format = CurrentFormat()) -> List[dict]: +def _validate_well_images( + images: List[Union[str, dict]], fmt: Format = CurrentFormat() +) -> List[dict]: VALID_KEYS = [ "acquisition", @@ -120,7 +122,9 @@ def _validate_plate_acquisitions( return acquisitions -def _validate_plate_wells(wells: List, fmt: Format = CurrentFormat()) -> List[dict]: +def _validate_plate_wells( + wells: List[Union[str, dict]], fmt: Format = CurrentFormat() +) -> List[dict]: VALID_KEYS = [ "path", @@ -225,7 +229,7 @@ def write_plate_metadata( group: zarr.Group, rows: List[str], columns: List[str], - wells: Union[List[str], List[dict]], + wells: List[Union[str, dict]], fmt: Format = CurrentFormat(), acquisitions: List[dict] = None, field_count: int = None, @@ -242,7 +246,7 @@ def write_plate_metadata( The list of names for the plate rows columns: list of str The list of names for the plate columns - wells: list of str or list of dict + wells: list of str or dict The list of paths for the well groups fmt: Format The format of the ome_zarr data which should be used. @@ -272,7 +276,7 @@ def write_plate_metadata( def write_well_metadata( group: zarr.Group, - images: Union[List[str], List[dict]], + images: List[Union[str, dict]], fmt: Format = CurrentFormat(), ) -> None: """ @@ -282,7 +286,7 @@ def write_well_metadata( ---------- group: zarr.Group the group within the zarr store to write the metadata in. - image_paths: list of str + image_paths: list of str or dict The list of paths for the well images image_acquisitions: list of int The list of acquisitions for the well images From 9f065af465e75b09e1f1460d053bf6a6b0064769 Mon Sep 17 00:00:00 2001 From: Sebastien Besson Date: Fri, 14 Jan 2022 14:57:59 +0000 Subject: [PATCH 7/7] Fix typo --- ome_zarr/writer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ome_zarr/writer.py b/ome_zarr/writer.py index 87f1ea58..b7786802 100644 --- a/ome_zarr/writer.py +++ b/ome_zarr/writer.py @@ -139,7 +139,7 @@ def _validate_plate_wells( if any(e not in VALID_KEYS for e in well.keys()): LOGGER.debug("f{well} contains unspecified keys") if "path" not in well: - raise ValueError(f"{well} must contain an path key") + raise ValueError(f"{well} must contain a path key") if not isinstance(well["path"], str): raise ValueError(f"{well} path must be of str type") validated_wells.append(well)