Merge pull request #466 from fractal-analytics-platform/266-fix-test_…

…manually_removing_overlap Fix `check_well_for_FOV_overlap` and `test_manually_removing_overlap`
fractal-analytics-platform · Jul 18, 2023 · dc4836d · dc4836d
2 parents e4727be + 61846e3
commit dc4836d
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,10 +4,11 @@
     * Relax `zarr` required version to `>=2.13.6,<3`;
     * Relax `pandas` required version to `>=1.2.0,<2`;
     * Relax `Pillow` required version to `>=9.1.1,<10.0.0`;
-* Update Zenodo datasets used in tests (\#454).
-* Improve coverage of core library (\#459).
+* Update default values for tolerance (`tol`) in `lib_ROI_overlaps.py` functions (\#466).
 * Internal changes (\#455):
-    * Update the `poetry.lock` version of several dependencies (`cellpose`, `dask`, `napari-skimage-regionprops`, `Pillow`, `scikit-image`, `zarr`).
+    * Improve coverage of core library (\#459).
+    * Update Zenodo datasets used in tests (\#454).
+    * Update the `poetry.lock` version of several dependencies (`cellpose`, `dask`, `napari-skimage-regionprops`, `Pillow`, `scikit-image`, `zarr`, `lxml`, `pydantic`).
     * Include `requests` and `wget` in the `dev` poetry dependency group.
     * Run tests both for the poetry- and pip-installed package.
     * Update generic type hints (\#462).

diff --git a/fractal_tasks_core/lib_ROI_overlaps.py b/fractal_tasks_core/lib_ROI_overlaps.py
@@ -26,7 +26,7 @@
 
 
 def is_overlapping_1D(
-    line1: Sequence[float], line2: Sequence[float], tol: float = 0
+    line1: Sequence[float], line2: Sequence[float], tol: float = 1e-10
 ) -> bool:
     """
     Given two intervals, finds whether they overlap
@@ -44,7 +44,7 @@ def is_overlapping_1D(
 
 
 def is_overlapping_2D(
-    box1: Sequence[float], box2: Sequence[float], tol: float = 0
+    box1: Sequence[float], box2: Sequence[float], tol: float = 1e-10
 ) -> bool:
     """
     Given two rectangular boxes, finds whether they overlap
@@ -67,7 +67,7 @@ def is_overlapping_2D(
     return overlap_x and overlap_y
 
 
-def is_overlapping_3D(box1, box2, tol=0) -> bool:
+def is_overlapping_3D(box1, box2, tol: float = 1e-10) -> bool:
     """
     Given two three-dimensional boxes, finds whether they overlap
 
@@ -94,7 +94,7 @@ def is_overlapping_3D(box1, box2, tol=0) -> bool:
 
 
 def get_overlapping_pair(
-    tmp_df: pd.DataFrame, tol: float = 0
+    tmp_df: pd.DataFrame, tol: float = 1e-10
 ) -> Union[tuple[int, int], bool]:
     """
     Finds the indices for the next overlapping FOVs pair
@@ -171,7 +171,7 @@ def apply_shift_in_one_direction(
     line_1: Sequence[float],
     line_2: Sequence[float],
     mu: str,
-    tol: float = 0,
+    tol: float = 1e-10,
 ):
     min_1, max_1 = line_1[:]
     min_2, max_2 = line_2[:]
@@ -393,7 +393,7 @@ def check_well_for_FOV_overlap(
     site_metadata: pd.DataFrame,
     selected_well: str,
     plotting_function: Callable,
-    tol: float = 0,
+    tol: float = 1e-10,
 ):
     """
     This function is currently only used in tests and examples.
@@ -442,7 +442,7 @@ def check_well_for_FOV_overlap(
 
 def run_overlap_check(
     site_metadata: pd.DataFrame,
-    tol: float = 0,
+    tol: float = 1e-10,
     plotting_function: Optional[Callable] = None,
 ):
     """

diff --git a/tests/data/metadata_files/overlapping_ROIs/MeasurementData.mlf b/tests/data/metadata_files/overlapping_ROIs/MeasurementData.mlf
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<bts:MeasurementData bts:Version="1.0" xmlns:bts="http://www.yokogawa.co.jp/BTS/BTSSchema/1.0">
+<bts:MeasurementRecord bts:Type="IMG" bts:Time="2020-08-12T17:36:36.234+02:00" bts:Column="3" bts:Row="2" bts:TimePoint="2" bts:FieldIndex="1" bts:ZIndex="1" bts:TimelineIndex="1" bts:ActionIndex="1" bts:Action="3D" bts:X="-1448.3" bts:Y="1517.7" bts:Z="-2.0" bts:Ch="1">myplate_B03_T0002F001L01A01Z01C01.tif</bts:MeasurementRecord>
+<bts:MeasurementRecord bts:Type="IMG" bts:Time="2020-08-12T17:36:36.234+02:00" bts:Column="3" bts:Row="2" bts:TimePoint="2" bts:FieldIndex="1" bts:ZIndex="2" bts:TimelineIndex="1" bts:ActionIndex="1" bts:Action="3D" bts:X="-1448.3" bts:Y="1517.7" bts:Z="-1.0" bts:Ch="1">myplate_B03_T0002F001L01A01Z02C01.tif</bts:MeasurementRecord>
+<bts:MeasurementRecord bts:Type="IMG" bts:Time="2020-08-12T17:36:46.322+02:00" bts:Column="3" bts:Row="2" bts:TimePoint="2" bts:FieldIndex="2" bts:ZIndex="1" bts:TimelineIndex="1" bts:ActionIndex="1" bts:Action="3D" bts:X="-1032.4" bts:Y="1517.7" bts:Z="-2.0" bts:Ch="1">myplate_B03_T0002F002L01A01Z01C01.tif</bts:MeasurementRecord>
+<bts:MeasurementRecord bts:Type="IMG" bts:Time="2020-08-12T17:36:46.322+02:00" bts:Column="3" bts:Row="2" bts:TimePoint="2" bts:FieldIndex="2" bts:ZIndex="2" bts:TimelineIndex="1" bts:ActionIndex="1" bts:Action="3D" bts:X="-1032.4" bts:Y="1517.7" bts:Z="-1.0" bts:Ch="1">myplate_B03_T0002F002L01A01Z02C01.tif</bts:MeasurementRecord>
+</bts:MeasurementData>
diff --git a/tests/data/metadata_files/overlapping_ROIs/corrected_site_metadata.csv b/tests/data/metadata_files/overlapping_ROIs/corrected_site_metadata.csv
@@ -0,0 +1,3 @@
+well_id,FieldIndex,x_micrometer,y_micrometer,z_micrometer,pixel_size_z,z_pixel,Time,pixel_size_x,pixel_size_y,x_pixel,y_pixel,bit_depth,x_micrometer_original,y_micrometer_original
+B03,1,-1448.3,-1517.7,0,1.0,2,2020-08-12 15:36:36.234000+00:00,0.1625,0.1625,2560,2160,16,-1448.3,-1517.7
+B03,2,-1032.3,-1517.7,0,1.0,2,2020-08-12 15:36:46.322000+00:00,0.1625,0.1625,2560,2160,16,-1032.4,-1517.7
diff --git a/tests/test_unit_parse_yokogawa_metadata.py b/tests/test_unit_parse_yokogawa_metadata.py
@@ -191,37 +191,46 @@ def test_parse_yokogawa_metadata_multiwell():
     assert file_numbers == {"C03": 4, "B03": 4, "D04": 8}
 
 
-def test_manually_removing_overlap():
-    import logging
-
-    logging.warning(
-        "test_manually_removing_overlap is relying on a wrong "
-        "behavior, i.e. the detection of overlaps due to tol=0. "
-        "To do: identify a DataFrame with an actual overlap and "
-        "update this test."
+def test_manually_removing_overlap(testdata_path):
+
+    # Set paths
+    mlf_path = str(
+        testdata_path
+        / "metadata_files"
+        / "overlapping_ROIs"
+        / "MeasurementData.mlf"
     )
-    # Tests the overlap detection and manually removing the overlaps
-    site_metadata, _ = parse_yokogawa_metadata(mrf_path_1, mlf_path_1)
+    mrf_path = str(testdata_path / "metadata_files" / "MeasurementDetail.mrf")
+    expected_site_metadata_path = str(
+        testdata_path
+        / "metadata_files"
+        / "overlapping_ROIs"
+        / "corrected_site_metadata.csv"
+    )
+
+    # Tests that overlaps are identified
+    site_metadata, _ = parse_yokogawa_metadata(mrf_path, mlf_path)
     site_metadata["x_micrometer_original"] = site_metadata["x_micrometer"]
     site_metadata["y_micrometer_original"] = site_metadata["y_micrometer"]
-    overlapping_FOVs = run_overlap_check(site_metadata, tol=0)
-
+    overlapping_FOVs = run_overlap_check(site_metadata)
     expected_overlaps = [{"B03": [2, 1]}]
     assert overlapping_FOVs == expected_overlaps
-    site_metadata.loc[("B03", 2), "x_micrometer"] = -1032.2
+
+    # Manually remove overlap (by hand)
+    site_metadata.loc[("B03", 2), "x_micrometer"] = -1032.3
 
     # Check that overlap has been successfully removed
-    overlapping_FOVs_empty = run_overlap_check(site_metadata, tol=1e-10)
+    overlapping_FOVs_empty = run_overlap_check(site_metadata)
     assert len(overlapping_FOVs_empty) == 0
 
     # Load expected dataframe and set index + types correctly
-    expected_site_metadata = pd.read_csv(
-        Path(path) / "corrected_site_metadata_tiny_test.csv"
-    )
+    expected_site_metadata = pd.read_csv(expected_site_metadata_path)
     expected_site_metadata.set_index(["well_id", "FieldIndex"], inplace=True)
     expected_site_metadata["Time"] = pd.to_datetime(
         expected_site_metadata["Time"]
     )
+
+    # Assert equality with expected dataframe
     pd.testing.assert_frame_equal(site_metadata, expected_site_metadata)