Update package #30
Update package #30
Conversation
| from .. import util, cache | ||
|
|
||
| @cache.cache_netcdf | ||
| def get_chd_at_model_edge(model_ds, idomain): |
There was a problem hiding this comment.
we should also add a method that does this based on a shape(file)?
There was a problem hiding this comment.
Yes, that would not be too hard. The function gdf_to_bool_data_array from the mgrid module can probably help with that.
There was a problem hiding this comment.
Can be done in as part of a new issue.
nlmod/util.py
Outdated
|
|
||
| 1. Read cached dataset and merge this with the current model_ds if all | ||
| of the following conditions are satisfied: | ||
| return model_ds[list(model_ds.coords)] |
There was a problem hiding this comment.
is this a copy, or is it still linked to the original data?
There was a problem hiding this comment.
Good question. I am not 100% sure but I think it is a copy. If I later add data variables to model_ds these are not added to the returned dataset from get_model_ds_empty. I also tried modifying the original coördinates of model_ds but could not do this. Therefore I think it is fine to get an empty model dataset using this code. To be sure we can also add .copy(deep=True) at the end. This is slower but not extremely slow, see timing below:
What do you think?
There was a problem hiding this comment.
Perhaps we can check by checking the memory id for the object, the dataarray and underlying array? id(model_ds["x"]) etc?
But I'm also fine with the .copy() for now just to be sure.
There was a problem hiding this comment.
The id's are the same but they differ everytime I call the id function which is a bit confusing. I will use copy() for now
There was a problem hiding this comment.
Looks good! Lots of nice changes I think! I think after tests pass it can be merged, but leaving some things i came across anyway:
Comments on notebooks:
- 01 contains reference to broken KNMI api, but this is fixed now i believe.
- 07 set extents equal in resampling comparison plots. (+typo in Rectengular)
- 07 some headings could use a bit more explanation, i.e. how it deals with ties (i.e. when 4 cells are equally close to new cell midpoint)
|
Also we should mention somewhere this is a breaking update for existing scripts relying on nlmod! |
|
Yes we should, I can mention it when I draft a new release |
Major update including: