Deltares / xugrid

Xarray and unstructured grids
https://deltares.github.io/xugrid/
MIT License
58 stars 8 forks source link

Make Ugrid topology objects as immutable as possible #195

Open Huite opened 8 months ago

Huite commented 8 months ago

Topology objects are currently propagated through operations. This is desirable, since their size can be relatively large, and we would be copying quite a lot of data potentially.

However, it means that "action at a distance" may give unexpected results: when changing some UgridDataArray b, where b is derived from a (through e.g. full_like), a is changed as well.

The easiest solution to this is to make the grid topology objects as immutable as possible. We should shield attributes such as node_x and node_y, and make them properties instead (like most of the other attributes). To support transformations of the geometry (which is currently technically possible through mutation), we should implement specific methods instead which return a new object.

set_crs should also be updated, since it currently mutates (something I wasn't sure about, it matches geopandas...) There might be some other methods which actually mutate.

Huite commented 8 months ago

To disallow the following:

grid2d.face_x += 10.0

I think I'd suggest adding the following method to AbstractUgrid:

    @staticmethod
    def _immutable_array_view(array: np.ndarray) -> np.ndarray:
        """
        Let's look at the following example, where face_x is a property returning a view on a numpy array:

        >>> grid.face_x -= 10.0

        This fails, since the face_x property does not have a setter. However, the minus operation does work.
        So the operation fails halfway.
        """
        array = array.view()
        array.flags.writeable = False
        return array
veenstrajelmer commented 3 months ago

Good idea, but maybe add an option to add custom attributes upon the generation of the Ugrid2d:

grid = xu.Ugrid2d(node_x=node_coords_x,
                  node_y=node_coords_y,
                  face_node_connectivity=face_node_connectivity,
                  fill_value=-1,
                  attrs={'vertical_dimensions': 'mesh2d_nLayers: mesh2d_nInterfaces (padding: none)'}
                  )