Do not add variables/attributes to a Dataset that is used as input

[OnnoEbbens]

There are some functions that (unexpectedly) modify the Dataset that is used as input for the function. Mostly because variables or attributes are added to the Dataset. For examples the functions. ds_to_rch, ds_to_evt and modelgrid_to_vertex_ds.

[rubencalje]

Another example is set_ds_time.

So should we start these methods with a copy of the Dataset?

[dbrakenhoff]

If the method has "set_" in the name, I think this is ok, as the function name is pretty clear that it's setting something? But good to review whether this is the desired behavior.

[OnnoEbbens]

Depends on the function. In the case of set_ds_time I would say yes because we want to return the full dataset. In case of ds_to_rch and ds_to_evt I think we don't have to add anything to the dataset. For modelgrid_to_vertex_ds I think we should also make a copy.

A few problems may arise when creating a copy. If you make a shallow copy you will modify the dataset anyway. Making a deep copy may be time consuming. So we should look into this when making a copy.

[OnnoEbbens]

If the method has "set_" in the name, I think this is ok, as the function name is pretty clear that it's setting something? But good to review whether this is the desired behavior.

I agree with this as well. So we should not create a copy for function starting with set_...

Now that I examined modelgrid_to_vertex_ds better I think this function has become obsolete since we have the modelgrid_to_ds function which already deals with a vertex dataset.

[rubencalje]

Yes , modelgrid_to_vertex_d can probably be removed. It could be used to add modelgrid-information to an existing Dataset (and not create a new Dataset like modelgrid_to_ds). I do not think we use it anywhere, and I do not see a use case for it.