Change .close_region(...) to modify internal dataframe

echoregions/regions2d/regions2d.py

@@ -386,12 +386,23 @@ def select_region(
 
         return region
 
+    def _close_regions_dataframe(self, region: DataFrame) -> DataFrame:
+        """Return a copy of a region-selection dataframe with its polygon closed."""
+        closed_region = region.copy()
+        closed_region.loc[:, "time"] = closed_region.apply(
+            lambda row: np.append(row["time"], row["time"][0]), axis=1
+        )
+        closed_region.loc[:, "depth"] = closed_region.apply(
+            lambda row: np.append(row["depth"], row["depth"][0]), axis=1
+        )
+        return closed_region
+
     def close_region(
         self,
         region_id: Union[float, int, str, List[Union[float, int, str]]] = None,
         region_class: Union[str, List[str]] = None,
-    ) -> DataFrame:
-        """Close a region by appending the first point to end of the list of points.
+    ) -> None:
+        """Close region(s) in the stored dataframe by appending the first point to the end.
 
         Parameters
         ----------
@@ -400,20 +411,14 @@ def close_region(
             If ``None``, select all regions. Defaults to ``None``
         region_class : Union[str, List[str]], ``None``
             A region class or a list of region classes.
-
-        Returns
-        -------
-        DataFrame
-            Returns a new DataFrame with closed regions
         """
         region = self.select_region(region_id, region_class, copy=True)
-        region.loc[:, "time"] = region.apply(
-            lambda row: np.append(row["time"], row["time"][0]), axis=1
-        )
-        region.loc[:, "depth"] = region.apply(
-            lambda row: np.append(row["depth"], row["depth"][0]), axis=1
-        )
-        return region
+        closed_region = self._close_regions_dataframe(region)
+        for idx in closed_region.index:
+            # use .at to change the values in the internal dataframe
+            # that contains the same indices as the closed region dataframe
+            self.data.at[idx, "time"] = closed_region.at[idx, "time"]
+            self.data.at[idx, "depth"] = closed_region.at[idx, "depth"]
 
     def select_sonar_file(
         self,
@@ -545,7 +550,8 @@ def plot(
 
         # Select Region(s)
         if close_regions:
-            region = self.close_region(region_id, region_class)
+            region = self.select_region(region_id, region_class, copy=True)
+            region = self._close_regions_dataframe(region)
         else:
             region = self.select_region(region_id, region_class)
         for _, row in region.iterrows():

echoregions/tests/test_regions2d.py

@@ -272,6 +272,36 @@ def test_evr_to_file(regions2d_fixture: Regions2D, tmp_path) -> None:
     assert os.path.exists(regions2d_fixture.output_file[0])
 
 
+@pytest.mark.regions2d
+def test_close_region_modifies_internal_dataframe(regions2d_fixture: Regions2D) -> None:
+    """Test that close_region modifies the internal Regions2D dataframe in place and returns None."""
+    regions2d_fixture.data = regions2d_fixture.data.copy()
+    region_before = regions2d_fixture.select_region([11], copy=False)
+    original_time = region_before.iloc[0]["time"]
+    original_depth = region_before.iloc[0]["depth"]
+    result = regions2d_fixture.close_region([11])
+    assert result is None
+    region_after = regions2d_fixture.select_region([11], copy=False)
+    assert len(region_after.iloc[0]["time"]) == len(original_time) + 1
+    assert len(region_after.iloc[0]["depth"]) == len(original_depth) + 1
+    assert region_after.iloc[0]["time"][0] == original_time[0]
+    assert region_after.iloc[0]["time"][-1] == original_time[0]
+    assert region_after.iloc[0]["depth"][0] == original_depth[0]
+    assert region_after.iloc[0]["depth"][-1] == original_depth[0]
+
+
+@pytest.mark.regions2d
+def test_plot_close_regions_does_not_modify_internal_dataframe(
+    regions2d_fixture: Regions2D,
+) -> None:
+    """Test that plotting with close_regions=True does not modify the internal dataframe."""
+    data_before = regions2d_fixture.data.copy(deep=True)
+
+    regions2d_fixture.plot([11], close_regions=True, color="k")
+
+    assert regions2d_fixture.data.equals(data_before)
+
+
 @pytest.mark.regions2d
 def test_plot(regions2d_fixture: Regions2D) -> None:
     """