Remapping Overwrites Original Data (#819)

* Initial Commit

* Added `destination_grid`

---------

Co-authored-by: Philip Chmielowiec <[email protected]>

test/test_remap.py

@@ -159,7 +159,21 @@ def test_edge_centers_remapping(self):
                                                                          remap_to="edge centers")
 
         # Assert the data variable lies on the "edge centers"
-        self.assertTrue(destination_grid['v1']._edge_centered())
+        self.assertTrue(remap_to_edge_centers['v1']._edge_centered())
+
+    def test_overwrite(self):
+        """Tests that the remapping no longer overwrites the dataset."""
+
+        # Open source and destination datasets to remap to
+        source_grid = ux.open_dataset(gridfile_geoflow, dsfile_v1_geoflow)
+        destination_dataset = ux.open_dataset(gridfile_geoflow, dsfile_v1_geoflow)
+
+        # Perform remapping
+        remap_to_edge_centers = source_grid['v1'].remap.nearest_neighbor(destination_obj=destination_dataset,
+                                                                                  remap_to="nodes")
+
+        # Assert the remapped data is different from the original data
+        assert not np.array_equal(destination_dataset['v1'], remap_to_edge_centers)
 
 
 class TestInverseDistanceWeightedRemapping(TestCase):
@@ -289,4 +303,18 @@ def test_edge_remapping(self):
                                                                                   remap_to="edge centers")
 
         # Assert the data variable lies on the "edge centers"
-        self.assertTrue(destination_grid['v1']._edge_centered())
+        self.assertTrue(remap_to_edge_centers['v1']._edge_centered())
+
+    def test_overwrite(self):
+        """Tests that the remapping no longer overwrites the dataset."""
+
+        # Open source and destination datasets to remap to
+        source_grid = ux.open_dataset(gridfile_geoflow, dsfile_v1_geoflow)
+        destination_dataset = ux.open_dataset(gridfile_geoflow, dsfile_v1_geoflow)
+
+        # Perform Remapping
+        remap_to_edge_centers = source_grid['v1'].remap.inverse_distance_weighted(destination_obj=destination_dataset,
+                                                                         remap_to="nodes")
+
+        # Assert the remapped data is different from the original data
+        assert not np.array_equal(destination_dataset['v1'], remap_to_edge_centers)

uxarray/remap/dataarray_accessor.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
-from typing import TYPE_CHECKING, Union
+from typing import TYPE_CHECKING, Optional
+from warnings import warn
 
 from uxarray.remap.nearest_neighbor import _nearest_neighbor_uxda
 from uxarray.remap.inverse_distance_weighted import (
@@ -30,7 +31,8 @@ def __repr__(self):
 
     def nearest_neighbor(
         self,
-        destination_obj: Union[Grid, UxDataArray, UxDataset],
+        destination_grid: Optional[Grid] = None,
+        destination_obj: Optional[Grid, UxDataArray, UxDataset] = None,
         remap_to: str = "nodes",
         coord_type: str = "spherical",
     ):
@@ -39,19 +41,40 @@ def nearest_neighbor(
 
         Parameters
         ---------
+        destination_grid : Grid
+            Destination Grid for remapping
         destination_obj : Grid, UxDataArray, UxDataset
-            Destination for remapping
+            Optional destination for remapping, deprecating
         remap_to : str, default="nodes"
             Location of where to map data, either "nodes" or "face centers"
         coord_type : str, default="spherical"
             Indicates whether to remap using on spherical or cartesian coordinates
         """
+        if destination_grid is not None and destination_obj is not None:
+            raise ValueError(
+                "Only one destination allowed, "
+                "please remove either `destination_grid` or `destination_obj`."
+            )
+        elif destination_grid is None and destination_obj is None:
+            raise ValueError("Destination needed for remap.")
 
-        return _nearest_neighbor_uxda(self.uxda, destination_obj, remap_to, coord_type)
+        if destination_grid is not None:
+            return _nearest_neighbor_uxda(
+                self.uxda, destination_grid, remap_to, coord_type
+            )
+        elif destination_obj is not None:
+            warn(
+                "destination_obj will be deprecated in a future release. Please use destination_grid instead.",
+                DeprecationWarning,
+            )
+            return _nearest_neighbor_uxda(
+                self.uxda, destination_obj, remap_to, coord_type
+            )
 
     def inverse_distance_weighted(
         self,
-        destination_obj: Union[Grid, UxDataArray, UxDataset],
+        destination_grid: Optional[Grid] = None,
+        destination_obj: Optional[Grid, UxDataArray, UxDataset] = None,
         remap_to: str = "nodes",
         coord_type: str = "spherical",
         power=2,
@@ -62,8 +85,10 @@ def inverse_distance_weighted(
 
         Parameters
         ---------
+        destination_grid : Grid
+            Destination Grid for remapping
         destination_obj : Grid, UxDataArray, UxDataset
-            Destination for remapping
+            Optional destination for remapping, deprecating
         remap_to : str, default="nodes"
             Location of where to map data, either "nodes" or "face centers"
         coord_type : str, default="spherical"
@@ -74,7 +99,23 @@ def inverse_distance_weighted(
         k : int, default=8
             Number of nearest neighbors to consider in the weighted calculation.
         """
+        if destination_grid is not None and destination_obj is not None:
+            raise ValueError(
+                "Only one destination allowed, "
+                "please remove either `destination_grid` or `destination_obj`."
+            )
+        elif destination_grid is None and destination_obj is None:
+            raise ValueError("Destination needed for remap.")
 
-        return _inverse_distance_weighted_remap_uxda(
-            self.uxda, destination_obj, remap_to, coord_type, power, k
-        )
+        if destination_grid is not None:
+            return _inverse_distance_weighted_remap_uxda(
+                self.uxda, destination_grid, remap_to, coord_type
+            )
+        elif destination_obj is not None:
+            warn(
+                "destination_obj will be deprecated in a future release. Please use destination_grid instead.",
+                DeprecationWarning,
+            )
+            return _inverse_distance_weighted_remap_uxda(
+                self.uxda, destination_obj, remap_to, coord_type
+            )

uxarray/remap/dataset_accessor.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
-from typing import TYPE_CHECKING, Union
+from typing import TYPE_CHECKING, Optional
+from warnings import warn
 
 from uxarray.remap.nearest_neighbor import _nearest_neighbor_uxds
 from uxarray.remap.inverse_distance_weighted import (
@@ -30,7 +31,8 @@ def __repr__(self):
 
     def nearest_neighbor(
         self,
-        destination_obj: Union[Grid, UxDataArray, UxDataset],
+        destination_grid: Optional[Grid] = None,
+        destination_obj: Optional[Grid, UxDataArray, UxDataset] = None,
         remap_to: str = "nodes",
         coord_type: str = "spherical",
     ):
@@ -39,19 +41,41 @@ def nearest_neighbor(
 
         Parameters
         ---------
+        destination_grid : Grid
+            Destination Grid for remapping
         destination_obj : Grid, UxDataArray, UxDataset
-            Destination for remapping
+            Optional destination for remapping, deprecating
         remap_to : str, default="nodes"
             Location of where to map data, either "nodes", "edge centers", or "face centers"
         coord_type : str, default="spherical"
             Indicates whether to remap using on spherical or cartesian coordinates
         """
 
-        return _nearest_neighbor_uxds(self.uxds, destination_obj, remap_to, coord_type)
+        if destination_grid is not None and destination_obj is not None:
+            raise ValueError(
+                "Only one destination allowed, "
+                "please remove either `destination_grid` or `destination_obj`."
+            )
+        elif destination_grid is None and destination_obj is None:
+            raise ValueError("Destination needed for remap.")
+
+        if destination_grid is not None:
+            return _nearest_neighbor_uxds(
+                self.uxds, destination_grid, remap_to, coord_type
+            )
+        elif destination_obj is not None:
+            warn(
+                "destination_obj will be deprecated in a future release. Please use destination_grid instead.",
+                DeprecationWarning,
+            )
+            return _nearest_neighbor_uxds(
+                self.uxds, destination_obj, remap_to, coord_type
+            )
 
     def inverse_distance_weighted(
         self,
-        destination_obj: Union[Grid, UxDataArray, UxDataset],
+        destination_grid: Optional[Grid] = None,
+        destination_obj: Optional[Grid, UxDataArray, UxDataset] = None,
         remap_to: str = "nodes",
         coord_type: str = "spherical",
         power=2,
@@ -62,8 +86,10 @@ def inverse_distance_weighted(
 
         Parameters
         ---------
+        destination_grid : Grid
+            Destination Grid for remapping
         destination_obj : Grid, UxDataArray, UxDataset
-            Destination for remapping
+            Optional destination for remapping, deprecating
         remap_to : str, default="nodes"
             Location of where to map data, either "nodes", "edge centers", or "face centers"
         coord_type : str, default="spherical"
@@ -75,6 +101,23 @@ def inverse_distance_weighted(
             Number of nearest neighbors to consider in the weighted calculation.
         """
 
-        return _inverse_distance_weighted_remap_uxds(
-            self.uxds, destination_obj, remap_to, coord_type, power, k
-        )
+        if destination_grid is not None and destination_obj is not None:
+            raise ValueError(
+                "Only one destination allowed, "
+                "please remove either `destination_grid` or `destination_obj`."
+            )
+        elif destination_grid is None and destination_obj is None:
+            raise ValueError("Destination needed for remap.")
+
+        if destination_grid is not None:
+            return _inverse_distance_weighted_remap_uxds(
+                self.uxds, destination_grid, remap_to, coord_type
+            )
+        elif destination_obj is not None:
+            warn(
+                "destination_obj will be deprecated in a future release. Please use destination_grid instead.",
+                DeprecationWarning,
+            )
+            return _inverse_distance_weighted_remap_uxds(
+                self.uxds, destination_obj, remap_to, coord_type
+            )

uxarray/remap/inverse_distance_weighted.py

@@ -231,12 +231,13 @@ def _inverse_distance_weighted_remap_uxda(
     )
     # add remapped variable to existing UxDataset
     if isinstance(destination_obj, uxarray.core.dataset.UxDataset):
-        destination_obj[source_uxda.name] = uxda_remap
-        return destination_obj
+        uxds = destination_obj.copy()
+        uxds[source_uxda.name] = uxda_remap
+        return uxds
 
     # construct a UxDataset from remapped variable and existing variable
     elif isinstance(destination_obj, uxarray.core.dataset.UxDataArray):
-        uxds = destination_obj.to_dataset()
+        uxds = destination_obj.copy().to_dataset()
         uxds[source_uxda.name] = uxda_remap
         return uxds
 

uxarray/remap/nearest_neighbor.py

@@ -199,12 +199,13 @@ def _nearest_neighbor_uxda(
     )
     # add remapped variable to existing UxDataset
     if isinstance(destination_obj, uxarray.core.dataset.UxDataset):
-        destination_obj[source_uxda.name] = uxda_remap
-        return destination_obj
+        uxds = destination_obj.copy()
+        uxds[source_uxda.name] = uxda_remap
+        return uxds
 
     # construct a UxDataset from remapped variable and existing variable
     elif isinstance(destination_obj, uxarray.core.dataset.UxDataArray):
-        uxds = destination_obj.to_dataset()
+        uxds = destination_obj.copy().to_dataset()
         uxds[source_uxda.name] = uxda_remap
         return uxds