Deltares / Deltares/imod-python
API inconsistency regarding layer coords and MetaSWAP ``.from_imod5_data`` methods
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 41
- Forks
- 12
- Avg merge
- 21h 8m
- Merged PRs (30d)
- 1
Description
https://github.com/Deltares/imod-python/pull/1335 will implement an imod.msw.MetaSwapModel.from_imod5_data method. What is not tackled here is the following:
imod.prj.open_projectfile_datareturns grids for the CAP package with a layer coordinate- This coordinate information can be potentially required for when coupling MetaSwapModels to other layers than layer 1 (when this is implemented)
- Right now,
imod.msw.MetaSwapModel.from_imod5_datadrops this information - The package.from_imod5_data implementations require the layer coord to be dropped
This makes for inconsistent API: imod.msw.MetaSwapModel.from_imod5_data is OK with the layer coord that is always added by open_projectfile_data, whereas e.g. imod.msw.GridData.from_imod5_data requires it to be dropped.
This makes for an annoying inconsistency in the API, for when users try to manually import individual packages themselves. I don't see a big use-case for it, but it IS unexpected behaviour.
To get something which works is very easy: xarray has an errors: "ignore" argument for its xr.DataArray.isel method. My concern is more a matter of performance: I haven't tested yet what xarray does when it ignores errors: Does it return a copy or a view? If the first, this would mean that potentially a lot of data would be copied, as the same grids are used for different packages, therefore grids would be copied multiple times for each package. Some profiling on the LHM data would be good to do here to test whether this leads to big bottlenecks.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing imod.prj.open_projectfile_data with the MetaSwapModel.from_imod5_data and package.from_imod5_data implementations, including imod.msw.GridData.from_imod5_data. Profile the LHM data path while testing xarray.DataArray.isel with errors="ignore". Done means the from_imod5_data APIs handle layer coordinates consistently without an unacceptable copying or performance cost.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100