Issue #963 dump meta swap model nc#1852
Conversation
…in utilities, accepting the IModel interface as argument. The MetaSWAP model now also implements this interface, so can be prepared to use the same functionality
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
Thanks, the changeset is quite complete and the roundtrip tests are good.
I have some comments though:
-
There is duplicate code in imod.mf6.Model.dump, which is never called now because imod.mf6.Modflow6Simulation doesn't call it anymore... See my detailed comments.
-
It's very useful you added the iModel interface to MetaSwapModel. You had to add some methods to comply with the iModel interface. Unfortunately the masking methods do not work as some important methods are missing. Implementing that in this PR is out of scope, but could you add a NotImplementedError here? Furthermore could you add unittests for the others that we still think work? If it turns out in the unittests that they do not work, also replace these with a NotImplmentedError. List of methods/properties that still deserve a unittest:
- _is_splitting_supported
- _is_regridding_supported
- _is_clipping_supported
- model_id
- options
- domain
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
Looks good, using the IDict interface simplified things a lot! I like the work you put into generalizing the dump logic into a common utility. I have one minor remark about the docstring, as that looked a bit fishy to me so please have a look at that. Approving in advance. Thanks!
| experimental option which creates a zipped zarr store in a single | ||
| file, which is easier to copy and automatically compresses data as | ||
| well. Default is ``'netcdf4'``. | ||
| Dump simulation to files. Writes a model definition as .TOML file, which |
There was a problem hiding this comment.
I see you added whitespace here. Was this necessary to get the docs rendered properly, or is by accident?
|
3 participants
Fixes #963
Description
Checklist
Issue #nr, e.g.Issue #737pixi run generate-sbomand committed changes