Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Ugrid topology objects as immutable as possible #195

Open
Huite opened this issue Dec 14, 2023 · 2 comments
Open

Make Ugrid topology objects as immutable as possible #195

Huite opened this issue Dec 14, 2023 · 2 comments

Comments

@Huite
Copy link
Collaborator

Huite commented Dec 14, 2023

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
Copy link
Collaborator Author

Huite commented Dec 14, 2023

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
Copy link
Collaborator

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)'}
                  )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants