search

gwmod / nlmod

Python package to build, run and visualize MODFLOW 6 groundwater models in the Netherlands.
https://nlmod.readthedocs.io
MIT License
34 stars 3 forks source link

add modelextent plot #357

Closed dbrakenhoff closed 4 months ago

dbrakenhoff commented 4 months ago

This PR is originally made by @dbrakenhoff to add a modelextent-plot.

It has been misused by me (@rubencalje) to replace some of the methods from the resample-module to the grid-module. For example the nlmod.resample.get_extent(ds) now is nlmod.grid.get_extent(ds), which I think is more logical. The original methods still function, and the user will see a warning that the old methods are deprecated.

dbrakenhoff commented 4 months ago

Test fails unrelated.

rubencalje commented 4 months ago

Nice addition. There are already two functions in nlmod.util to create a polygon from the extent polygon_from_extent and to create a geodataframe from an extent gdf_from_extent. I think these functions are nice to keep and the plot function can call them. Only thing is that we should move them somewhere else because 'util' is not the best place. Probably the resample module is a good place because get_extent is also in there.

It is even worse... We also have nlmod.resample.get_extent_polygon. This method also takes a rotated grid into account. Maybe we can merge some of these methods?

rubencalje commented 4 months ago

Nice addition. There are already two functions in nlmod.util to create a polygon from the extent polygon_from_extent and to create a geodataframe from an extent gdf_from_extent. I think these functions are nice to keep and the plot function can call them. Only thing is that we should move them somewhere else because 'util' is not the best place. Probably the resample module is a good place because get_extent is also in there.

It is even worse... We also have nlmod.resample.get_extent_polygon. This method also takes a rotated grid into account. Maybe we can merge some of these methods?

I moved some of the methods from the resample-module to the grid-module (also see the starting-message of this PR). I also moved extent_to_polygon from the resample-module to the util-module, where it replaces polygon_from_extent, and renamed gdf_from_extent to extent_to_gdf in the util-module. These methods in the util-module can also be used without a grid, and therefore the util-module is a good place for these methods.

All the old methods continue to work for now, and the user will get to see a deprecation-message.

dbrakenhoff commented 4 months ago

Test fails are unrelated to the last commit. Maybe something to do with the new KNMI stations containing old data? One of the examples is looking for precipitation data from 1893...

Merging this PR, if we need some more fixes, we'll figure that out for the new release PR I'll open shortly.