From 91e7fdd4b41263ec40752dcad3bcdf7ec848211c Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sun, 7 Jul 2024 12:39:54 -0700 Subject: [PATCH 01/26] Updated rename wells functionality at CLI level, fixed style errors --- iohub/cli/cli.py | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ iohub/ngff.py | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 901b92a9..ed70585e 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -1,9 +1,11 @@ +import csv import pathlib import click from iohub._version import __version__ from iohub.convert import TIFFConverter +from iohub.ngff import open_ome_zarr from iohub.reader import print_info VERSION = __version__ @@ -87,3 +89,60 @@ def convert(input, output, grid_layout, chunks): chunks=chunks, ) converter() + + +@click.command() +@click.help_option("-h", "--help") +@click.argument( + "csvfile", + type=click.File("r"), +) +@click.argument( + "zarrfile", + type=click.Path( + exists=True, + file_okay=True, + dir_okay=False, + resolve_path=True, + path_type=pathlib.Path, + ), +) +def rename_wells_cli(csvfile, zarrfile): + """Rename wells based on CSV file + + The CSV file should have two columns: old_well_path and new_well_path. + """ + names = [] + + csvreader = csv.reader(csvfile) + for row in csvreader: + if len(row) != 2: + raise ValueError( + f"Invalid row format: {row}." + f"Each row must have two columns." + ) + names.append([row[0], row[1]]) + + plate = open_ome_zarr(zarrfile, mode="a") + + modified = {} + modified["wells"] = [] + modified["rows"] = [] + modified["columns"] = [] + + well_paths = [ + plate.metadata.wells[i].path for i in range(len(plate.metadata.wells)) + ] + + for old_well_path, new_well_path in names: + if old_well_path not in well_paths: + raise ValueError( + f"Old well path '{old_well_path}' not found " + f"in the plate metadata." + ) + for well in plate.metadata.wells: + if well.path == old_well_path and well not in modified["wells"]: + plate.rename_well( + plate, well, old_well_path, new_well_path, modified, False + ) + modified["wells"].append(well) diff --git a/iohub/ngff.py b/iohub/ngff.py index d5a307e0..698215c5 100644 --- a/iohub/ngff.py +++ b/iohub/ngff.py @@ -1565,6 +1565,50 @@ def positions(self) -> Generator[tuple[str, Position], None, None]: for _, position in well.positions(): yield position.zgroup.path, position + def rename_well( + self, + well, + old_well_path: str, + new_well_path: str, + modified={}, + single_well=True, + ): + """Rename a well. + + Parameters + ---------- + old_well_path : str + Old name of well + new_well_path : str + New name of well + modified : dict, optional + Tracks modified wells + single well: bool, optional + True: renaming one well, False: renaming multiple wells + """ + old_row, old_column = old_well_path.split("/") + new_row, new_column = new_well_path.split("/") + + if well.path == old_well_path: + well.path = new_well_path # update well metadata + zarr.storage.rename( + self.zgroup._store, old_well_path, new_well_path + ) # update well paths + + for column in self.metadata.columns: + if column.name == old_column and column not in modified["columns"]: + column.name = new_column # update column metadata + if not single_well: + modified["columns"].append(column) + + for row in self.metadata.rows: + if row.name == old_row and row not in modified["rows"]: + row.name = new_row # update row metadata + if not single_well: + modified["rows"].append(row) + + self.dump_meta() + def open_ome_zarr( store_path: StrOrBytesPath, From d336cd85997be29043c3c2489104b8bc7fd1d247 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Tue, 9 Jul 2024 11:43:18 -0700 Subject: [PATCH 02/26] CLI changes --- iohub/cli/cli.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index ed70585e..fef0b69b 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -91,23 +91,23 @@ def convert(input, output, grid_layout, chunks): converter() -@click.command() +@cli.command() @click.help_option("-h", "--help") -@click.argument( - "csvfile", - type=click.File("r"), +@click.option( + "-i", + "input.zarr", + type=click.Path(exists=True, file_okay=True, dir_okay=False), + required=True, + help="Path to the input Zarr file.", ) -@click.argument( - "zarrfile", - type=click.Path( - exists=True, - file_okay=True, - dir_okay=False, - resolve_path=True, - path_type=pathlib.Path, - ), +@click.option( + "-c", + "names.csv", + type=click.File("r"), + required=True, + help="Path to the CSV file containing well names.", ) -def rename_wells_cli(csvfile, zarrfile): +def rename_wells(csvfile, zarrfile): """Rename wells based on CSV file The CSV file should have two columns: old_well_path and new_well_path. @@ -143,6 +143,6 @@ def rename_wells_cli(csvfile, zarrfile): for well in plate.metadata.wells: if well.path == old_well_path and well not in modified["wells"]: plate.rename_well( - plate, well, old_well_path, new_well_path, modified, False + well, old_well_path, new_well_path, modified, False ) modified["wells"].append(well) From e3f70298cbbf4ac0ddd0164c3ce30f0cbd0e328e Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Tue, 9 Jul 2024 16:50:24 -0700 Subject: [PATCH 03/26] fix click options --- iohub/cli/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index fef0b69b..260e2ad9 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -95,14 +95,14 @@ def convert(input, output, grid_layout, chunks): @click.help_option("-h", "--help") @click.option( "-i", - "input.zarr", + "--input", type=click.Path(exists=True, file_okay=True, dir_okay=False), required=True, help="Path to the input Zarr file.", ) @click.option( "-c", - "names.csv", + "--csv", type=click.File("r"), required=True, help="Path to the CSV file containing well names.", From 4fb8b03c9cd75f8c8b4114840e69c11175d8a181 Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Tue, 9 Jul 2024 16:55:39 -0700 Subject: [PATCH 04/26] basic test for help message --- tests/cli/test_cli.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index b8ddf9c7..c6e8ede0 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -104,3 +104,13 @@ def test_cli_convert_ome_tiff(grid_layout, tmpdir): result = runner.invoke(cli, cmd) assert result.exit_code == 0, result.output assert "Converting" in result.output + + +def test_rename_wells_help(): + runner = CliRunner() + cmd = ["rename-wells"] + for option in ("-h", "--help"): + cmd.append(option) + result = runner.invoke(cli, cmd) + assert result.exit_code == 0 + assert "containing well names" in result.output From 1f27a194d2f1bf6adb064c5c2d92a882b6acbb43 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Thu, 11 Jul 2024 13:57:21 -0700 Subject: [PATCH 05/26] Updated decorator --- iohub/cli/cli.py | 9 +++++---- iohub/ngff.py | 38 ++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 260e2ad9..b01b0f67 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -108,6 +108,10 @@ def convert(input, output, grid_layout, chunks): help="Path to the CSV file containing well names.", ) def rename_wells(csvfile, zarrfile): + rename_wells_cli(csvfile, zarrfile) + + +def rename_wells_cli(csvfile, zarrfile): """Rename wells based on CSV file The CSV file should have two columns: old_well_path and new_well_path. @@ -125,10 +129,7 @@ def rename_wells(csvfile, zarrfile): plate = open_ome_zarr(zarrfile, mode="a") - modified = {} - modified["wells"] = [] - modified["rows"] = [] - modified["columns"] = [] + modified = [] well_paths = [ plate.metadata.wells[i].path for i in range(len(plate.metadata.wells)) diff --git a/iohub/ngff.py b/iohub/ngff.py index 698215c5..460e00fb 100644 --- a/iohub/ngff.py +++ b/iohub/ngff.py @@ -1570,8 +1570,6 @@ def rename_well( well, old_well_path: str, new_well_path: str, - modified={}, - single_well=True, ): """Rename a well. @@ -1589,25 +1587,29 @@ def rename_well( old_row, old_column = old_well_path.split("/") new_row, new_column = new_well_path.split("/") - if well.path == old_well_path: - well.path = new_well_path # update well metadata - zarr.storage.rename( - self.zgroup._store, old_well_path, new_well_path - ) # update well paths + well_paths = [well.path for well in self.metadata.wells] - for column in self.metadata.columns: - if column.name == old_column and column not in modified["columns"]: - column.name = new_column # update column metadata - if not single_well: - modified["columns"].append(column) + if old_well_path not in well_paths: + raise ValueError(f"Well '{old_well_path}' not found in plate.") - for row in self.metadata.rows: - if row.name == old_row and row not in modified["rows"]: - row.name = new_row # update row metadata - if not single_well: - modified["rows"].append(row) + else: + well = self[old_well_path] - self.dump_meta() + if well.path == old_well_path: + well.path = new_well_path # update well metadata + zarr.storage.rename( + self.zgroup._store, old_well_path, new_well_path + ) # update well paths + + for column in self.metadata.columns: + if column.name == old_column: + column.name = new_column # update column metadata + + for row in self.metadata.rows: + if row.name == old_row: + row.name = new_row # update row metadata + + self.dump_meta() def open_ome_zarr( From 66e5d12c41640b98bca206bfe04da3472aacfb6a Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Thu, 11 Jul 2024 16:06:48 -0700 Subject: [PATCH 06/26] Exception changes --- iohub/cli/cli.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index b01b0f67..52f5dca5 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -131,19 +131,13 @@ def rename_wells_cli(csvfile, zarrfile): modified = [] - well_paths = [ - plate.metadata.wells[i].path for i in range(len(plate.metadata.wells)) - ] - for old_well_path, new_well_path in names: - if old_well_path not in well_paths: - raise ValueError( - f"Old well path '{old_well_path}' not found " - f"in the plate metadata." - ) for well in plate.metadata.wells: - if well.path == old_well_path and well not in modified["wells"]: - plate.rename_well( - well, old_well_path, new_well_path, modified, False - ) - modified["wells"].append(well) + if well.path == old_well_path and well not in modified: + try: + plate.rename_well( + well, old_well_path, new_well_path, modified, False + ) + modified.append(well) + except ValueError as e: + click.echo(f"Error: {e}", err=True) From 75a9327043476d2e8673bc5188976d5d4c9da128 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Fri, 12 Jul 2024 11:17:40 -0700 Subject: [PATCH 07/26] Updated rename wells and testing --- iohub/cli/cli.py | 18 ++++++++++++------ iohub/ngff.py | 10 ++++------ tests/cli/test_cli.py | 32 +++++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 52f5dca5..993d3864 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -96,13 +96,15 @@ def convert(input, output, grid_layout, chunks): @click.option( "-i", "--input", - type=click.Path(exists=True, file_okay=True, dir_okay=False), + "zarrfile", + type=click.Path(exists=True, file_okay=True, dir_okay=True), required=True, help="Path to the input Zarr file.", ) @click.option( "-c", "--csv", + "csvfile", type=click.File("r"), required=True, help="Path to the CSV file containing well names.", @@ -116,6 +118,7 @@ def rename_wells_cli(csvfile, zarrfile): The CSV file should have two columns: old_well_path and new_well_path. """ + names = [] csvreader = csv.reader(csvfile) @@ -131,13 +134,16 @@ def rename_wells_cli(csvfile, zarrfile): modified = [] - for old_well_path, new_well_path in names: + print(f"names: {names}") + + for name in names: + old_well_path, new_well_path = name[0], name[1] for well in plate.metadata.wells: - if well.path == old_well_path and well not in modified: + if str(well.path) == str(old_well_path) and well not in modified: + print(f"Renaming {old_well_path} to {new_well_path}...") try: - plate.rename_well( - well, old_well_path, new_well_path, modified, False - ) + plate.rename_well(well, old_well_path, new_well_path) modified.append(well) + print(f"Well {old_well_path} renamed to {new_well_path}") except ValueError as e: click.echo(f"Error: {e}", err=True) diff --git a/iohub/ngff.py b/iohub/ngff.py index 460e00fb..23c143ee 100644 --- a/iohub/ngff.py +++ b/iohub/ngff.py @@ -1579,10 +1579,6 @@ def rename_well( Old name of well new_well_path : str New name of well - modified : dict, optional - Tracks modified wells - single well: bool, optional - True: renaming one well, False: renaming multiple wells """ old_row, old_column = old_well_path.split("/") new_row, new_column = new_well_path.split("/") @@ -1592,8 +1588,10 @@ def rename_well( if old_well_path not in well_paths: raise ValueError(f"Well '{old_well_path}' not found in plate.") - else: - well = self[old_well_path] + elif old_well_path in well_paths: + well = next( + w for w in self.metadata.wells if w.path == old_well_path + ) if well.path == old_well_path: well.path = new_well_path # update well metadata diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index c6e8ede0..19e980db 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -1,17 +1,17 @@ +import csv import re from unittest.mock import patch -from click.testing import CliRunner import pytest +from click.testing import CliRunner from iohub._version import __version__ from iohub.cli.cli import cli - from tests.conftest import ( + hcs_ref, mm2gamma_ome_tiffs, ndtiff_v2_datasets, ndtiff_v3_labeled_positions, - hcs_ref, ) @@ -114,3 +114,29 @@ def test_rename_wells_help(): result = runner.invoke(cli, cmd) assert result.exit_code == 0 assert "containing well names" in result.output + + +def test_rename_wells_basic(): + runner = CliRunner() + test_zarr = "/hpc/mydata/joseph.schull/stitched_phase.zarr" + test_csv = "/hpc/mydata/joseph.schull/update_well_names.csv" + cmd = ["rename-wells", "-i", test_zarr, "-c", test_csv] + result = runner.invoke(cli, cmd) + print(result.output) + assert result.exit_code == 0 + + +def test_rename_wells_completion(): + runner = CliRunner() + test_zarr = "/hpc/mydata/joseph.schull/stitched_phase.zarr" + test_csv = "/hpc/mydata/joseph.schull/update_well_names.csv" + cmd = ["rename-wells", "-i", test_zarr, "-c", test_csv] + result = runner.invoke(cli, cmd) + with open(test_csv, mode="r") as infile: + reader = csv.reader(infile) + names = list(reader) + for oldname, newname in names: + expected_message = f"Well {oldname} renamed to {newname}" + assert ( + expected_message in result.output + ), f"Missing expected message: {expected_message}" From f256cfe55ba3956d312f1896a962ec58263ea00c Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Fri, 12 Jul 2024 11:34:38 -0700 Subject: [PATCH 08/26] Updated test --- iohub/cli/cli.py | 3 +-- tests/cli/test_cli.py | 9 --------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 993d3864..399c3e65 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -136,8 +136,7 @@ def rename_wells_cli(csvfile, zarrfile): print(f"names: {names}") - for name in names: - old_well_path, new_well_path = name[0], name[1] + for old_well_path, new_well_path in names: for well in plate.metadata.wells: if str(well.path) == str(old_well_path) and well not in modified: print(f"Renaming {old_well_path} to {new_well_path}...") diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 19e980db..8af74943 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -122,16 +122,7 @@ def test_rename_wells_basic(): test_csv = "/hpc/mydata/joseph.schull/update_well_names.csv" cmd = ["rename-wells", "-i", test_zarr, "-c", test_csv] result = runner.invoke(cli, cmd) - print(result.output) assert result.exit_code == 0 - - -def test_rename_wells_completion(): - runner = CliRunner() - test_zarr = "/hpc/mydata/joseph.schull/stitched_phase.zarr" - test_csv = "/hpc/mydata/joseph.schull/update_well_names.csv" - cmd = ["rename-wells", "-i", test_zarr, "-c", test_csv] - result = runner.invoke(cli, cmd) with open(test_csv, mode="r") as infile: reader = csv.reader(infile) names = list(reader) From 65a039498a3a5638384c70d74b0359db568137b9 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sun, 14 Jul 2024 17:00:00 -0700 Subject: [PATCH 09/26] Updated test --- iohub/cli/cli.py | 11 +++++++-- tests/cli/test_cli.py | 57 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 399c3e65..8d445caf 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -132,9 +132,11 @@ def rename_wells_cli(csvfile, zarrfile): plate = open_ome_zarr(zarrfile, mode="a") - modified = [] + print( + f"Initial well paths: {[well.path for well in plate.metadata.wells]}" + ) - print(f"names: {names}") + modified = [] for old_well_path, new_well_path in names: for well in plate.metadata.wells: @@ -146,3 +148,8 @@ def rename_wells_cli(csvfile, zarrfile): print(f"Well {old_well_path} renamed to {new_well_path}") except ValueError as e: click.echo(f"Error: {e}", err=True) + + print( + f"Process completed" + f"Final well paths: {[well.path for well in plate.metadata.wells]}" + ) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 8af74943..ae422054 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -116,11 +116,60 @@ def test_rename_wells_help(): assert "containing well names" in result.output -def test_rename_wells_basic(): +def test_rename_wells_basic(tmpdir): runner = CliRunner() - test_zarr = "/hpc/mydata/joseph.schull/stitched_phase.zarr" - test_csv = "/hpc/mydata/joseph.schull/update_well_names.csv" - cmd = ["rename-wells", "-i", test_zarr, "-c", test_csv] + test_csv = tmpdir / "well_names.csv" + csv_data = [ + ["B/03", "B/03modified"], + ] + with open(test_csv, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data) + + cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv)] + result = runner.invoke(cli, cmd) + + print(result.output) + assert result.exit_code == 0 + + final_well_paths = None + + for line in result.output.split("\n"): + if line.startswith("Process completed. Final well paths:"): + final_well_paths = eval(line.split(": ")[1]) + break + + assert ( + final_well_paths is not None + ), "Final well paths not found in the output" + + new_well_paths = [row[1] for row in csv_data] + old_well_paths = [row[0] for row in csv_data] + + for new_path in new_well_paths: + assert ( + new_path in final_well_paths + ), f"Expected {new_path} in final well paths" + + for old_path in old_well_paths: + assert ( + old_path not in final_well_paths + ), f"Did not expect {old_path} in final well paths" + + +def test_rename_wells_renaming_message(tmpdir): + runner = CliRunner() + test_csv = tmpdir / "well_names.csv" + + csv_data = [ + ["B/03", "B/03"], + ] + + with open(test_csv, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data) + + cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv)] result = runner.invoke(cli, cmd) assert result.exit_code == 0 with open(test_csv, mode="r") as infile: From 01132dcb819f4d8b6ad77e178f330ef834cb84cd Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sun, 14 Jul 2024 17:20:49 -0700 Subject: [PATCH 10/26] test_rename_wells_basic --- iohub/cli/cli.py | 5 +---- tests/cli/test_cli.py | 29 ++--------------------------- 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 8d445caf..00e87888 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -149,7 +149,4 @@ def rename_wells_cli(csvfile, zarrfile): except ValueError as e: click.echo(f"Error: {e}", err=True) - print( - f"Process completed" - f"Final well paths: {[well.path for well in plate.metadata.wells]}" - ) + print(f"Final well paths: {[well.path for well in plate.metadata.wells]}") diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index ae422054..349f20ae 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -120,7 +120,7 @@ def test_rename_wells_basic(tmpdir): runner = CliRunner() test_csv = tmpdir / "well_names.csv" csv_data = [ - ["B/03", "B/03modified"], + ["B/03", "B/03test"], ] with open(test_csv, mode="w", newline="") as csvfile: writer = csv.writer(csvfile) @@ -135,7 +135,7 @@ def test_rename_wells_basic(tmpdir): final_well_paths = None for line in result.output.split("\n"): - if line.startswith("Process completed. Final well paths:"): + if line.startswith("Final well paths:"): final_well_paths = eval(line.split(": ")[1]) break @@ -155,28 +155,3 @@ def test_rename_wells_basic(tmpdir): assert ( old_path not in final_well_paths ), f"Did not expect {old_path} in final well paths" - - -def test_rename_wells_renaming_message(tmpdir): - runner = CliRunner() - test_csv = tmpdir / "well_names.csv" - - csv_data = [ - ["B/03", "B/03"], - ] - - with open(test_csv, mode="w", newline="") as csvfile: - writer = csv.writer(csvfile) - writer.writerows(csv_data) - - cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv)] - result = runner.invoke(cli, cmd) - assert result.exit_code == 0 - with open(test_csv, mode="r") as infile: - reader = csv.reader(infile) - names = list(reader) - for oldname, newname in names: - expected_message = f"Well {oldname} renamed to {newname}" - assert ( - expected_message in result.output - ), f"Missing expected message: {expected_message}" From c7c42edddc75abc35c4ff6c542a281e086496782 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sun, 14 Jul 2024 18:23:18 -0700 Subject: [PATCH 11/26] Updated tests, converting well names back to original --- tests/cli/test_cli.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 349f20ae..f9720cf6 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -116,7 +116,7 @@ def test_rename_wells_help(): assert "containing well names" in result.output -def test_rename_wells_basic(tmpdir): +def test_rename_wells(tmpdir): runner = CliRunner() test_csv = tmpdir / "well_names.csv" csv_data = [ @@ -155,3 +155,15 @@ def test_rename_wells_basic(tmpdir): assert ( old_path not in final_well_paths ), f"Did not expect {old_path} in final well paths" + + test_csv_2 = tmpdir / "well_names_2.csv" + csv_data = [ + ["B/03test", "B/03"], + ] + with open(test_csv_2, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data) + + cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv_2)] + result = runner.invoke(cli, cmd) + print(result.output) From 452f8ed010d8febf3dfc1a4afb436cf849e68afb Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sun, 14 Jul 2024 19:58:06 -0700 Subject: [PATCH 12/26] Context manager update --- iohub/cli/cli.py | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 00e87888..723e368b 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -130,23 +130,28 @@ def rename_wells_cli(csvfile, zarrfile): ) names.append([row[0], row[1]]) - plate = open_ome_zarr(zarrfile, mode="a") - - print( - f"Initial well paths: {[well.path for well in plate.metadata.wells]}" - ) - - modified = [] - - for old_well_path, new_well_path in names: - for well in plate.metadata.wells: - if str(well.path) == str(old_well_path) and well not in modified: - print(f"Renaming {old_well_path} to {new_well_path}...") - try: - plate.rename_well(well, old_well_path, new_well_path) - modified.append(well) - print(f"Well {old_well_path} renamed to {new_well_path}") - except ValueError as e: - click.echo(f"Error: {e}", err=True) - - print(f"Final well paths: {[well.path for well in plate.metadata.wells]}") + with open_ome_zarr(zarrfile, mode="a") as plate: + well_paths = [well.path for well in plate.metadata.wells] + print(f"Initial well paths: {well_paths}") + + modified = [] + + for old_well_path, new_well_path in names: + for well in plate.metadata.wells: + if ( + str(well.path) == str(old_well_path) + and well not in modified + ): + print(f"Renaming {old_well_path} to {new_well_path}...") + try: + plate.rename_well(well, old_well_path, new_well_path) + modified.append(well) + print( + f"Well {old_well_path} renamed to {new_well_path}" + ) + except ValueError as e: + click.echo(f"Error: {e}", err=True) + + print( + f"Final well paths: {[well.path for well in plate.metadata.wells]}" + ) From c753a87fbc1f29bc8c11f464376a4fa6bab97a77 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Wed, 17 Jul 2024 16:39:36 -0700 Subject: [PATCH 13/26] well-mapping.csv --- docs/examples/well-mapping.csv | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docs/examples/well-mapping.csv diff --git a/docs/examples/well-mapping.csv b/docs/examples/well-mapping.csv new file mode 100644 index 00000000..6ebe5570 --- /dev/null +++ b/docs/examples/well-mapping.csv @@ -0,0 +1,4 @@ +0/0,0/A +0/1,0/B +0/2,0/C +0/3,0/D From 6851360beb9e05ea76d278f2b1fc98cab81f0563 Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sat, 20 Jul 2024 19:43:47 -0700 Subject: [PATCH 14/26] Updating row and column indices --- iohub/ngff.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/iohub/ngff.py b/iohub/ngff.py index 23c143ee..8f4e5e5b 100644 --- a/iohub/ngff.py +++ b/iohub/ngff.py @@ -1,4 +1,4 @@ -# TODO: remove this in the future (PEP deferred for 3.11, now 3.12?) +c# TODO: remove this in the future (PEP deferred for 3.11, now 3.12?) from __future__ import annotations import logging @@ -1595,6 +1595,8 @@ def rename_well( if well.path == old_well_path: well.path = new_well_path # update well metadata + well.row_index = new_row + well.column_index = new_column zarr.storage.rename( self.zgroup._store, old_well_path, new_well_path ) # update well paths From ce70316c2359dac6fd78de39c7a48a4276a3cf1e Mon Sep 17 00:00:00 2001 From: Joseph Schull Date: Sat, 20 Jul 2024 19:47:48 -0700 Subject: [PATCH 15/26] Style changes --- iohub/ngff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/iohub/ngff.py b/iohub/ngff.py index 8f4e5e5b..11113aa0 100644 --- a/iohub/ngff.py +++ b/iohub/ngff.py @@ -1,4 +1,4 @@ -c# TODO: remove this in the future (PEP deferred for 3.11, now 3.12?) +# TODO: remove this in the future (PEP deferred for 3.11, now 3.12?) from __future__ import annotations import logging @@ -1595,7 +1595,7 @@ def rename_well( if well.path == old_well_path: well.path = new_well_path # update well metadata - well.row_index = new_row + well.row_index = new_row well.column_index = new_column zarr.storage.rename( self.zgroup._store, old_well_path, new_well_path From 31888c6d6761a6e7dad621d78eb8fddd087a3e46 Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 10:13:47 -0700 Subject: [PATCH 16/26] rework API with `self.zgroup.move` --- iohub/ngff/nodes.py | 74 +++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/iohub/ngff/nodes.py b/iohub/ngff/nodes.py index ee9070fa..965df379 100644 --- a/iohub/ngff/nodes.py +++ b/iohub/ngff/nodes.py @@ -1575,49 +1575,51 @@ def positions(self) -> Generator[tuple[str, Position], None, None]: def rename_well( self, - well, - old_well_path: str, - new_well_path: str, + old: str, + new: str, ): - """Rename a well. + """Rename a well. Parameters ---------- - old_well_path : str - Old name of well - new_well_path : str - New name of well + old : str + Old name of well, e.g. "A/1" + new : str + New name of well, e.g. "B/2" """ - old_row, old_column = old_well_path.split("/") - new_row, new_column = new_well_path.split("/") - - well_paths = [well.path for well in self.metadata.wells] - - if old_well_path not in well_paths: - raise ValueError(f"Well '{old_well_path}' not found in plate.") - - elif old_well_path in well_paths: - well = next( - w for w in self.metadata.wells if w.path == old_well_path - ) - - if well.path == old_well_path: - well.path = new_well_path # update well metadata - well.row_index = new_row - well.column_index = new_column - zarr.storage.rename( - self.zgroup._store, old_well_path, new_well_path - ) # update well paths - - for column in self.metadata.columns: - if column.name == old_column: - column.name = new_column # update column metadata - for row in self.metadata.rows: - if row.name == old_row: - row.name = new_row # update row metadata + old_row, old_column = old.split("/") + new_row, new_column = new.split("/") + + # raises ValueError if old well does not exist + # or if new well already exists + self.zgroup.move(old, new) + + # update well metadata + old_well_index = [well_name.path for well_name in self.metadata.wells].index(old) + self.metadata.wells[old_well_index].path = new + new_well_names = [well.path for well in self.metadata.wells] + + # update row/col metadata + # check for new row/col + if new_row not in [row.name for row in self.metadata.rows]: + self.metadata.rows.append(PlateAxisMeta(name=new_row)) + if new_column not in [col.name for col in self.metadata.columns]: + self.metadata.columns.append(PlateAxisMeta(name=new_column)) + + # check for empty row/col + if old_row not in [well.split("/")[0] for well in new_well_names]: + # delete empty row from zarr + del self.zgroup[old_row] + self.metadata.rows = [ + row for row in self.metadata.rows if row.name != old_row + ] + if old_column not in [well.split("/")[1] for well in new_well_names]: + self.metadata.columns = [ + col for col in self.metadata.columns if col.name != old_column + ] - self.dump_meta() + self.dump_meta() def open_ome_zarr( From 5b59dec0417482b66d59c7adcc1b397f9063534e Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 10:14:11 -0700 Subject: [PATCH 17/26] test API --- tests/ngff/test_ngff.py | 77 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/ngff/test_ngff.py b/tests/ngff/test_ngff.py index d1701529..1bc3436d 100644 --- a/tests/ngff/test_ngff.py +++ b/tests/ngff/test_ngff.py @@ -182,6 +182,39 @@ def _temp_ome_zarr( dataset.close() temp_dir.cleanup() +@contextmanager +def _temp_ome_zarr_plate( + image_5d: NDArray, channel_names: list[str], arr_name: str, position_list: list[tuple[str, str, str]], **kwargs +): + """Helper function to generate a temporary OME-Zarr store. + + Parameters + ---------- + image_5d : NDArray + channel_names : list[str] + arr_name : str + position_list : list[tuple[str, str, str]] + + Yields + ------ + Position + """ + try: + temp_dir = TemporaryDirectory() + dataset = open_ome_zarr( + os.path.join(temp_dir.name, "ome.zarr"), + layout="hcs", + mode="a", + channel_names=channel_names, + ) + for position in position_list: + pos = dataset.create_position(position[0], position[1], position[2]) + pos.create_image(arr_name, image_5d, **kwargs) + yield dataset + finally: + dataset.close() + temp_dir.cleanup() + @given( channels_and_random_5d=_channels_and_random_5d(), @@ -290,6 +323,50 @@ def test_rename_channel(channels_and_random_5d, arr_name, new_channel): assert new_channel in dataset.channel_names assert dataset.metadata.omero.channels[0].label == new_channel +@given( + channels_and_random_5d=_channels_and_random_5d(), + arr_name=short_alpha_numeric, +) +def test_rename_well(channels_and_random_5d, arr_name): + """Test `iohub.ngff.Position.rename_well()`""" + channel_names, random_5d = channels_and_random_5d + + position_list = [["A", "1", "0"], ["C", "4", "0"]] + with _temp_ome_zarr_plate(random_5d, channel_names, arr_name, position_list) as dataset: + assert dataset.zgroup["A/1"] + with pytest.raises(KeyError): + dataset.zgroup["B/2"] + assert "A" in [r[0] for r in dataset.rows()] + assert "B" not in [r[0] for r in dataset.rows()] + assert "A" in [row.name for row in dataset.metadata.rows] + assert "B" not in [row.name for row in dataset.metadata.rows] + assert "1" in [col.name for col in dataset.metadata.columns] + assert "2" not in [col.name for col in dataset.metadata.columns] + assert "C" in [row.name for row in dataset.metadata.rows] + assert "4" in [col.name for col in dataset.metadata.columns] + + dataset.rename_well("A/1", "B/2") + + assert dataset.zgroup["B/2"] + with pytest.raises(KeyError): + dataset.zgroup["A/1"] + assert "A" not in [r[0] for r in dataset.rows()] + assert "B" in [r[0] for r in dataset.rows()] + assert "A" not in [row.name for row in dataset.metadata.rows] + assert "B" in [row.name for row in dataset.metadata.rows] + assert "1" not in [col.name for col in dataset.metadata.columns] + assert "2" in [col.name for col in dataset.metadata.columns] + assert "C" in [row.name for row in dataset.metadata.rows] + assert "4" in [col.name for col in dataset.metadata.columns] + + # destination exists + with pytest.raises(ValueError): + dataset.rename_well("B/2", "C/4") + + # source doesn't exist + with pytest.raises(ValueError): + dataset.rename_well("Q/1", "Q/2") + @given( channels_and_random_5d=_channels_and_random_5d(), From fb0685a4936a032de710923079c32e28fbdd84da Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 10:14:33 -0700 Subject: [PATCH 18/26] simplify and document CLI --- iohub/cli/cli.py | 53 +++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 723e368b..2a851b2d 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -107,20 +107,22 @@ def convert(input, output, grid_layout, chunks): "csvfile", type=click.File("r"), required=True, - help="Path to the CSV file containing well names.", + help="Path to the CSV file containing old and new well names.", ) -def rename_wells(csvfile, zarrfile): - rename_wells_cli(csvfile, zarrfile) +def rename_wells(zarrfile, csvfile): + """Rename wells in an plate. + >> iohub rename-wells -i plate.zarr -c names.csv -def rename_wells_cli(csvfile, zarrfile): - """Rename wells based on CSV file - - The CSV file should have two columns: old_well_path and new_well_path. + The CSV file must have two columns with old and new names in the form: + ``` + A/1,B/2 + A/2,B/2 + ``` """ - names = [] - + # read and check csv + name_pair_list = [] csvreader = csv.reader(csvfile) for row in csvreader: if len(row) != 2: @@ -128,30 +130,13 @@ def rename_wells_cli(csvfile, zarrfile): f"Invalid row format: {row}." f"Each row must have two columns." ) - names.append([row[0], row[1]]) + name_pair_list.append([row[0], row[1]]) + # rename each well while catching errors with open_ome_zarr(zarrfile, mode="a") as plate: - well_paths = [well.path for well in plate.metadata.wells] - print(f"Initial well paths: {well_paths}") - - modified = [] - - for old_well_path, new_well_path in names: - for well in plate.metadata.wells: - if ( - str(well.path) == str(old_well_path) - and well not in modified - ): - print(f"Renaming {old_well_path} to {new_well_path}...") - try: - plate.rename_well(well, old_well_path, new_well_path) - modified.append(well) - print( - f"Well {old_well_path} renamed to {new_well_path}" - ) - except ValueError as e: - click.echo(f"Error: {e}", err=True) - - print( - f"Final well paths: {[well.path for well in plate.metadata.wells]}" - ) + for old, new in name_pair_list: + print(f"Renaming {old} to {new}") + try: + plate.rename_well(old, new) + except ValueError as e: + print(f"Error renaming {old} to {new}: {e}") \ No newline at end of file From 410f482dff9224b2af8eb6ac2a020c15cb9867db Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 10:14:55 -0700 Subject: [PATCH 19/26] test CLI w/ round trip --- tests/cli/test_cli.py | 107 ++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index f9720cf6..ef8c94dc 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -5,6 +5,7 @@ import pytest from click.testing import CliRunner +from iohub import open_ome_zarr from iohub._version import __version__ from iohub.cli.cli import cli from tests.conftest import ( @@ -14,6 +15,8 @@ ndtiff_v3_labeled_positions, ) +from ..ngff.test_ngff import _temp_copy + def pytest_generate_tests(metafunc): if "mm2gamma_ome_tiff" in metafunc.fixturenames: @@ -106,64 +109,66 @@ def test_cli_convert_ome_tiff(grid_layout, tmpdir): assert "Converting" in result.output -def test_rename_wells_help(): +def test_cli_rename_wells_help(): runner = CliRunner() cmd = ["rename-wells"] for option in ("-h", "--help"): cmd.append(option) result = runner.invoke(cli, cmd) assert result.exit_code == 0 - assert "containing well names" in result.output + assert ">> iohub rename-wells" in result.output -def test_rename_wells(tmpdir): - runner = CliRunner() - test_csv = tmpdir / "well_names.csv" - csv_data = [ - ["B/03", "B/03test"], - ] - with open(test_csv, mode="w", newline="") as csvfile: - writer = csv.writer(csvfile) - writer.writerows(csv_data) - - cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv)] - result = runner.invoke(cli, cmd) +def test_cli_rename_wells(tmpdir): + with _temp_copy(hcs_ref) as store_path: + runner = CliRunner() + test_csv = tmpdir / "well_names.csv" + csv_data = [ + ["B/03", "D/4"], + ] + with open(test_csv, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data) + + cmd = ["rename-wells", "-i", str(store_path), "-c", str(test_csv)] + result = runner.invoke(cli, cmd) + + assert result.exit_code == 0 + assert "Renaming" in result.output + + with open_ome_zarr(store_path, mode="r") as plate: + well_names = [well[0] for well in plate.wells()] + assert "D/4" in well_names + assert "B/03" not in well_names + assert len(plate.metadata.wells) == 1 + assert len(plate.metadata.rows) == 1 + assert len(plate.metadata.columns) == 1 + assert plate.metadata.wells[0].path == "D/4" + assert plate.metadata.rows[0].name == "D" + assert plate.metadata.columns[0].name == "4" + + # Test round trip + test_csv_2 = tmpdir / "well_names_2.csv" + csv_data = [ + ["D/4", "B/03"], + ] + with open(test_csv_2, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data) + + cmd = ["rename-wells", "-i", store_path, "-c", str(test_csv_2)] + result = runner.invoke(cli, cmd) + + with open_ome_zarr(store_path, mode="r") as plate: + well_names = [well[0] for well in plate.wells()] + assert "D/4" not in well_names + assert "B/03" in well_names + assert len(plate.metadata.wells) == 1 + assert len(plate.metadata.rows) == 1 + assert len(plate.metadata.columns) == 1 + assert plate.metadata.wells[0].path == "B/03" + assert plate.metadata.rows[0].name == "B" + assert plate.metadata.columns[0].name == "03" + - print(result.output) - assert result.exit_code == 0 - final_well_paths = None - - for line in result.output.split("\n"): - if line.startswith("Final well paths:"): - final_well_paths = eval(line.split(": ")[1]) - break - - assert ( - final_well_paths is not None - ), "Final well paths not found in the output" - - new_well_paths = [row[1] for row in csv_data] - old_well_paths = [row[0] for row in csv_data] - - for new_path in new_well_paths: - assert ( - new_path in final_well_paths - ), f"Expected {new_path} in final well paths" - - for old_path in old_well_paths: - assert ( - old_path not in final_well_paths - ), f"Did not expect {old_path} in final well paths" - - test_csv_2 = tmpdir / "well_names_2.csv" - csv_data = [ - ["B/03test", "B/03"], - ] - with open(test_csv_2, mode="w", newline="") as csvfile: - writer = csv.writer(csvfile) - writer.writerows(csv_data) - - cmd = ["rename-wells", "-i", hcs_ref, "-c", str(test_csv_2)] - result = runner.invoke(cli, cmd) - print(result.output) From 5caa25751a545db6f62eab3fc22159284b0a7fd0 Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 10:29:50 -0700 Subject: [PATCH 20/26] style --- iohub/cli/cli.py | 2 +- iohub/ngff/nodes.py | 12 +++++++----- tests/cli/test_cli.py | 3 --- tests/ngff/test_ngff.py | 16 +++++++++++++--- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 2a851b2d..7243f67b 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -139,4 +139,4 @@ def rename_wells(zarrfile, csvfile): try: plate.rename_well(old, new) except ValueError as e: - print(f"Error renaming {old} to {new}: {e}") \ No newline at end of file + print(f"Error renaming {old} to {new}: {e}") diff --git a/iohub/ngff/nodes.py b/iohub/ngff/nodes.py index 965df379..dba6edaf 100644 --- a/iohub/ngff/nodes.py +++ b/iohub/ngff/nodes.py @@ -1578,7 +1578,7 @@ def rename_well( old: str, new: str, ): - """Rename a well. + """Rename a well. Parameters ---------- @@ -1590,18 +1590,20 @@ def rename_well( old_row, old_column = old.split("/") new_row, new_column = new.split("/") - + # raises ValueError if old well does not exist # or if new well already exists self.zgroup.move(old, new) # update well metadata - old_well_index = [well_name.path for well_name in self.metadata.wells].index(old) + old_well_index = [ + well_name.path for well_name in self.metadata.wells + ].index(old) self.metadata.wells[old_well_index].path = new new_well_names = [well.path for well in self.metadata.wells] # update row/col metadata - # check for new row/col + # check for new row/col if new_row not in [row.name for row in self.metadata.rows]: self.metadata.rows.append(PlateAxisMeta(name=new_row)) if new_column not in [col.name for col in self.metadata.columns]: @@ -1610,7 +1612,7 @@ def rename_well( # check for empty row/col if old_row not in [well.split("/")[0] for well in new_well_names]: # delete empty row from zarr - del self.zgroup[old_row] + del self.zgroup[old_row] self.metadata.rows = [ row for row in self.metadata.rows if row.name != old_row ] diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index ef8c94dc..32ffd6fc 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -169,6 +169,3 @@ def test_cli_rename_wells(tmpdir): assert plate.metadata.wells[0].path == "B/03" assert plate.metadata.rows[0].name == "B" assert plate.metadata.columns[0].name == "03" - - - diff --git a/tests/ngff/test_ngff.py b/tests/ngff/test_ngff.py index 1bc3436d..8ea8786c 100644 --- a/tests/ngff/test_ngff.py +++ b/tests/ngff/test_ngff.py @@ -182,9 +182,14 @@ def _temp_ome_zarr( dataset.close() temp_dir.cleanup() + @contextmanager def _temp_ome_zarr_plate( - image_5d: NDArray, channel_names: list[str], arr_name: str, position_list: list[tuple[str, str, str]], **kwargs + image_5d: NDArray, + channel_names: list[str], + arr_name: str, + position_list: list[tuple[str, str, str]], + **kwargs, ): """Helper function to generate a temporary OME-Zarr store. @@ -208,7 +213,9 @@ def _temp_ome_zarr_plate( channel_names=channel_names, ) for position in position_list: - pos = dataset.create_position(position[0], position[1], position[2]) + pos = dataset.create_position( + position[0], position[1], position[2] + ) pos.create_image(arr_name, image_5d, **kwargs) yield dataset finally: @@ -323,6 +330,7 @@ def test_rename_channel(channels_and_random_5d, arr_name, new_channel): assert new_channel in dataset.channel_names assert dataset.metadata.omero.channels[0].label == new_channel + @given( channels_and_random_5d=_channels_and_random_5d(), arr_name=short_alpha_numeric, @@ -332,7 +340,9 @@ def test_rename_well(channels_and_random_5d, arr_name): channel_names, random_5d = channels_and_random_5d position_list = [["A", "1", "0"], ["C", "4", "0"]] - with _temp_ome_zarr_plate(random_5d, channel_names, arr_name, position_list) as dataset: + with _temp_ome_zarr_plate( + random_5d, channel_names, arr_name, position_list + ) as dataset: assert dataset.zgroup["A/1"] with pytest.raises(KeyError): dataset.zgroup["B/2"] From d1f763919259e1b589e08f6b729eef64c054069c Mon Sep 17 00:00:00 2001 From: Talon Chandler Date: Thu, 19 Sep 2024 11:17:21 -0700 Subject: [PATCH 21/26] make example typical --- docs/examples/well-mapping.csv | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/examples/well-mapping.csv b/docs/examples/well-mapping.csv index 6ebe5570..da66e067 100644 --- a/docs/examples/well-mapping.csv +++ b/docs/examples/well-mapping.csv @@ -1,4 +1,4 @@ -0/0,0/A -0/1,0/B -0/2,0/C -0/3,0/D +0/0,A/1 +0/1,A/2 +0/2,B/1 +0/3,B/2 From 2c0c083e99db6bd87fa70c62005a8ca66d92d157 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Mon, 30 Sep 2024 19:02:59 -0700 Subject: [PATCH 22/26] move rename_wells to its own file --- iohub/cli/cli.py | 30 ++++---------------- iohub/rename_wells.py | 57 ++++++++++++++++++++++++++++++++++++++ tests/cli/test_cli.py | 52 +++++----------------------------- tests/conftest.py | 26 +++++++++++++++++ tests/test_rename_wells.py | 32 +++++++++++++++++++++ 5 files changed, 127 insertions(+), 70 deletions(-) create mode 100644 iohub/rename_wells.py create mode 100644 tests/test_rename_wells.py diff --git a/iohub/cli/cli.py b/iohub/cli/cli.py index 7243f67b..0c409a98 100644 --- a/iohub/cli/cli.py +++ b/iohub/cli/cli.py @@ -1,12 +1,11 @@ -import csv import pathlib import click from iohub._version import __version__ from iohub.convert import TIFFConverter -from iohub.ngff import open_ome_zarr from iohub.reader import print_info +from iohub.rename_wells import rename_wells VERSION = __version__ @@ -91,7 +90,7 @@ def convert(input, output, grid_layout, chunks): converter() -@cli.command() +@cli.command(name="rename-wells") @click.help_option("-h", "--help") @click.option( "-i", @@ -105,11 +104,11 @@ def convert(input, output, grid_layout, chunks): "-c", "--csv", "csvfile", - type=click.File("r"), + type=click.Path(exists=True, file_okay=True, dir_okay=False), required=True, help="Path to the CSV file containing old and new well names.", ) -def rename_wells(zarrfile, csvfile): +def rename_wells_command(zarrfile, csvfile): """Rename wells in an plate. >> iohub rename-wells -i plate.zarr -c names.csv @@ -120,23 +119,4 @@ def rename_wells(zarrfile, csvfile): A/2,B/2 ``` """ - - # read and check csv - name_pair_list = [] - csvreader = csv.reader(csvfile) - for row in csvreader: - if len(row) != 2: - raise ValueError( - f"Invalid row format: {row}." - f"Each row must have two columns." - ) - name_pair_list.append([row[0], row[1]]) - - # rename each well while catching errors - with open_ome_zarr(zarrfile, mode="a") as plate: - for old, new in name_pair_list: - print(f"Renaming {old} to {new}") - try: - plate.rename_well(old, new) - except ValueError as e: - print(f"Error renaming {old} to {new}: {e}") + rename_wells(zarrfile, csvfile) diff --git a/iohub/rename_wells.py b/iohub/rename_wells.py new file mode 100644 index 00000000..887f78b7 --- /dev/null +++ b/iohub/rename_wells.py @@ -0,0 +1,57 @@ +import csv +from pathlib import Path + +from iohub.ngff import open_ome_zarr + + +def rename_wells(zarr_store_path: str | Path, csv_file_path: str | Path): + """ + Rename wells in a Zarr store based on a CSV file containing old and new + well names. + + Parameters + ---------- + zarr_store_path : str or Path + Path to the Zarr store. + csv_file_path : str or Path + Path to the CSV file containing the old and new well names. + + Raises + ------ + ValueError + If a row in the CSV file does not have exactly two columns. + If there is an error renaming a well in the Zarr store. + + Notes + ----- + The CSV file should have two columns: + - The first column contains the old well names. + - The second column contains the new well names. + + Examples + -------- + CSV file content: + A/1,B/2 + A/2,B/2 + + """ + + # read and check csv + name_pair_list = [] + with open(csv_file_path, "r") as csv_file: + for row in csv.reader(csv_file): + if len(row) != 2: + raise ValueError( + f"Invalid row format: {row}." + f"Each row must have two columns." + ) + name_pair_list.append([row[0], row[1]]) + + # rename each well while catching errors + with open_ome_zarr(zarr_store_path, mode="a") as plate: + for old, new in name_pair_list: + print(f"Renaming {old} to {new}") + try: + plate.rename_well(old, new) + except ValueError as e: + print(f"Error renaming {old} to {new}: {e}") diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 32ffd6fc..c464795b 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -1,11 +1,9 @@ -import csv import re from unittest.mock import patch import pytest from click.testing import CliRunner -from iohub import open_ome_zarr from iohub._version import __version__ from iohub.cli.cli import cli from tests.conftest import ( @@ -119,53 +117,17 @@ def test_cli_rename_wells_help(): assert ">> iohub rename-wells" in result.output -def test_cli_rename_wells(tmpdir): +def test_cli_rename_wells(csv_data_file_1): with _temp_copy(hcs_ref) as store_path: runner = CliRunner() - test_csv = tmpdir / "well_names.csv" - csv_data = [ - ["B/03", "D/4"], + cmd = [ + "rename-wells", + "-i", + str(store_path), + "-c", + str(csv_data_file_1), ] - with open(test_csv, mode="w", newline="") as csvfile: - writer = csv.writer(csvfile) - writer.writerows(csv_data) - - cmd = ["rename-wells", "-i", str(store_path), "-c", str(test_csv)] result = runner.invoke(cli, cmd) assert result.exit_code == 0 assert "Renaming" in result.output - - with open_ome_zarr(store_path, mode="r") as plate: - well_names = [well[0] for well in plate.wells()] - assert "D/4" in well_names - assert "B/03" not in well_names - assert len(plate.metadata.wells) == 1 - assert len(plate.metadata.rows) == 1 - assert len(plate.metadata.columns) == 1 - assert plate.metadata.wells[0].path == "D/4" - assert plate.metadata.rows[0].name == "D" - assert plate.metadata.columns[0].name == "4" - - # Test round trip - test_csv_2 = tmpdir / "well_names_2.csv" - csv_data = [ - ["D/4", "B/03"], - ] - with open(test_csv_2, mode="w", newline="") as csvfile: - writer = csv.writer(csvfile) - writer.writerows(csv_data) - - cmd = ["rename-wells", "-i", store_path, "-c", str(test_csv_2)] - result = runner.invoke(cli, cmd) - - with open_ome_zarr(store_path, mode="r") as plate: - well_names = [well[0] for well in plate.wells()] - assert "D/4" not in well_names - assert "B/03" in well_names - assert len(plate.metadata.wells) == 1 - assert len(plate.metadata.rows) == 1 - assert len(plate.metadata.columns) == 1 - assert plate.metadata.wells[0].path == "B/03" - assert plate.metadata.rows[0].name == "B" - assert plate.metadata.columns[0].name == "03" diff --git a/tests/conftest.py b/tests/conftest.py index 8d49c563..015a4951 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,9 @@ +import csv import shutil from pathlib import Path import fsspec +import pytest from wget import download @@ -100,3 +102,27 @@ def subdirs(parent: Path, name: str) -> list[Path]: ndtiff_v3_labeled_positions = test_datasets / "ndtiff_v3_labeled_positions" + + +@pytest.fixture +def csv_data_file_1(tmpdir): + test_csv_1 = tmpdir / "well_names_1.csv" + csv_data_1 = [ + ["B/03", "D/4"], + ] + with open(test_csv_1, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data_1) + return test_csv_1 + + +@pytest.fixture +def csv_data_file_2(tmpdir): + test_csv_2 = tmpdir / "well_names_2.csv" + csv_data_2 = [ + ["D/4", "B/03"], + ] + with open(test_csv_2, mode="w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerows(csv_data_2) + return test_csv_2 diff --git a/tests/test_rename_wells.py b/tests/test_rename_wells.py new file mode 100644 index 00000000..1df94f21 --- /dev/null +++ b/tests/test_rename_wells.py @@ -0,0 +1,32 @@ +from iohub import open_ome_zarr +from iohub.rename_wells import rename_wells +from tests.conftest import hcs_ref +from tests.ngff.test_ngff import _temp_copy + + +def test_cli_rename_wells(csv_data_file_1, csv_data_file_2): + with _temp_copy(hcs_ref) as store_path: + rename_wells(store_path, csv_data_file_1) + with open_ome_zarr(store_path, mode="r") as plate: + well_names = [well[0] for well in plate.wells()] + assert "D/4" in well_names + assert "B/03" not in well_names + assert len(plate.metadata.wells) == 1 + assert len(plate.metadata.rows) == 1 + assert len(plate.metadata.columns) == 1 + assert plate.metadata.wells[0].path == "D/4" + assert plate.metadata.rows[0].name == "D" + assert plate.metadata.columns[0].name == "4" + + # Test round trip + rename_wells(store_path, csv_data_file_2) + with open_ome_zarr(store_path, mode="r") as plate: + well_names = [well[0] for well in plate.wells()] + assert "D/4" not in well_names + assert "B/03" in well_names + assert len(plate.metadata.wells) == 1 + assert len(plate.metadata.rows) == 1 + assert len(plate.metadata.columns) == 1 + assert plate.metadata.wells[0].path == "B/03" + assert plate.metadata.rows[0].name == "B" + assert plate.metadata.columns[0].name == "03" From eb40820a72490aaa67bb393cdc345329164022a1 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Mon, 30 Sep 2024 19:22:09 -0700 Subject: [PATCH 23/26] add checks for correct well names --- iohub/ngff/nodes.py | 9 +++++++-- iohub/rename_wells.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/iohub/ngff/nodes.py b/iohub/ngff/nodes.py index dba6edaf..acfaac16 100644 --- a/iohub/ngff/nodes.py +++ b/iohub/ngff/nodes.py @@ -1588,8 +1588,13 @@ def rename_well( New name of well, e.g. "B/2" """ + # normalize inputs + old = normalize_storage_path(old) + new = normalize_storage_path(new) old_row, old_column = old.split("/") new_row, new_column = new.split("/") + new_row_meta = PlateAxisMeta(name=new_row) + new_col_meta = PlateAxisMeta(name=new_column) # raises ValueError if old well does not exist # or if new well already exists @@ -1605,9 +1610,9 @@ def rename_well( # update row/col metadata # check for new row/col if new_row not in [row.name for row in self.metadata.rows]: - self.metadata.rows.append(PlateAxisMeta(name=new_row)) + self.metadata.rows.append(new_row_meta) if new_column not in [col.name for col in self.metadata.columns]: - self.metadata.columns.append(PlateAxisMeta(name=new_column)) + self.metadata.columns.append(new_col_meta) # check for empty row/col if old_row not in [well.split("/")[0] for well in new_well_names]: diff --git a/iohub/rename_wells.py b/iohub/rename_wells.py index 887f78b7..a6ff018e 100644 --- a/iohub/rename_wells.py +++ b/iohub/rename_wells.py @@ -45,7 +45,7 @@ def rename_wells(zarr_store_path: str | Path, csv_file_path: str | Path): f"Invalid row format: {row}." f"Each row must have two columns." ) - name_pair_list.append([row[0], row[1]]) + name_pair_list.append([row[0].strip(), row[1].strip()]) # rename each well while catching errors with open_ome_zarr(zarr_store_path, mode="a") as plate: From f9b6847e174d1d4e231444eabe4c4036e156c6e9 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Mon, 30 Sep 2024 19:37:41 -0700 Subject: [PATCH 24/26] add tests for invalid well names --- tests/ngff/test_ngff.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/ngff/test_ngff.py b/tests/ngff/test_ngff.py index 8b546976..39c42f07 100644 --- a/tests/ngff/test_ngff.py +++ b/tests/ngff/test_ngff.py @@ -377,6 +377,12 @@ def test_rename_well(channels_and_random_5d, arr_name): with pytest.raises(ValueError): dataset.rename_well("Q/1", "Q/2") + # invalid well names + with pytest.raises(ValueError): + dataset.rename_well("B/2", " A/1") + with pytest.raises(ValueError): + dataset.rename_well("B/2", "A/?") + @given( channels_and_random_5d=_channels_and_random_5d(), From 27cda8d9a1cccd995c27b1f0597f9acba32bf6c1 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Thu, 3 Oct 2024 09:27:57 -0700 Subject: [PATCH 25/26] remove test_rename_wells deadline --- tests/ngff/test_ngff.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ngff/test_ngff.py b/tests/ngff/test_ngff.py index 35fae3ab..aff06a7d 100644 --- a/tests/ngff/test_ngff.py +++ b/tests/ngff/test_ngff.py @@ -335,6 +335,7 @@ def test_rename_channel(channels_and_random_5d, arr_name, new_channel): channels_and_random_5d=_channels_and_random_5d(), arr_name=short_alpha_numeric, ) +@settings(deadline=None) def test_rename_well(channels_and_random_5d, arr_name): """Test `iohub.ngff.Position.rename_well()`""" channel_names, random_5d = channels_and_random_5d From 33a61a414dcb7822f3c583ab59d2e2fb12587554 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Thu, 3 Oct 2024 15:16:17 -0700 Subject: [PATCH 26/26] remove test_set_scale deadline --- tests/ngff/test_ngff.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ngff/test_ngff.py b/tests/ngff/test_ngff.py index aff06a7d..4ec371a4 100644 --- a/tests/ngff/test_ngff.py +++ b/tests/ngff/test_ngff.py @@ -500,6 +500,7 @@ def test_set_transform_fov(ch_shape_dtype, arr_name): @given( ch_shape_dtype=_channels_and_random_5d_shape_and_dtype(), ) +@settings(deadline=None) def test_set_scale(ch_shape_dtype): channel_names, shape, dtype = ch_shape_dtype transform = [