Discussion: towards consistent API

[bdestombe]

Let us use this issue as a discussion board.

• Storing boundary condition information in geojsons
  • Currently I am storing most of the boundary condition information in geosjons as they are git-trackable.
  • Maybe we can publish under the gwtransport umbrella example geosjons for each boundary condition that are guaranteed to support all functionality of nlmod.
  • Validation functions that verify the geojson is valid
• For every boundary condition (RIV, DRN, HFB, LAK, etc) a XXX_from_gdf() function should exist.
  • gdf is used inconsistently here in nlmod, it is used for both gidded and ungridded geodataframes. My wish would be to use gdf for the ungridded geodataframes and that the XXX_from_gdf() functions do the gridding and aggregation. This allows us to use the data from the bullet above directly.
  • A module (XXX.py) should exist for every module in gwf
• The surfacewater module is a special beast. I think this module should be split. Part of this code would move to the riv.py and drn.py modules, and the other part to nlmod.read.waterboards. Such that nlmod.read.waterboards exposes an ungridded gdf that can be fed to the riv_from_gdf and drn_from_gdf from the riv.py and drn.py modules.

Probably these points are a bit short sighted, but I am very interested in your take on this. Maybe it is time to book select a date for heidedagen in Noordwijk

[dbrakenhoff]

Ew, consistent APIs 😉

My random first thoughts about this:

• Some surface water logic is general for all DRN/RIV/GHB/CHD packages, this should probably continue to live in surface_water.py. But the module should be cleaned up and all the data reading should definitely be moved to the read module.
• GeoDataFrames are used for all shapefiles, either gridded or not. Gridded shapefiles are used as list-based input, to avoid building sparse arrays and allow duplicates in cells. nlmod.grid.gdf_to_grid() is a pretty clear method for doing the gridding. The next step is optional aggregation which should I think be done by nlmod.grid.aggregate_vector_by_cell() which allows specifying aggregation options per column. A lot of the surface_water.py stuff can be replaced by this latter method I think? Though we need to think about supporting the De Lange upscaling (and then the most recent update of that method) in that the aggregation function as well.
• When gridding shapefiles, choices have to be made how to aggregate or not aggregate. This step has been kept separate because it requires a user choice for which it is not really possible to define a single good default? So I think it might be challenging to think of logical enough defaults to make this step "automatic" when building packages from source data. So maybe those settings should be required inputs, or just performed separately with the methods mentioned above.
• Definitely a good idea to write X_from_gdf() functions for each BC pkg. But we will have to think about how to deal with aggregation or not in that step.

the gwtransport umbrella

What is the gwtransport umbrella?

Validation functions that verify the geojson is valid

Good idea, also functions for spatially showing where certain data is missing is really useful.

Maybe it is time to book select a date for heidedagen in Noordwijk

😎 👍

[bdestombe]

gwtransport => gwmod. I was thinking it would not have to be part of this repository as its structure would change much slower than that of nlmod. Instead to make a separate repo that is part of the gwmod organization.