Issue #1320 Add CopyFiles package #1321
Conversation
imod/msw/copy_files.py
Outdated
| instead.""" | ||
|
|
||
|
|
||
| class CopyFiles(MetaSwapPackage): |
There was a problem hiding this comment.
Why does this class inherit from the MetaSwapPackage?
It seems like a really general class. I don't know how it is being used but is it possible to use composition over inheritance?
If it is possible to have this class without MetaSwapPackage as its parent class then you also don't need xarray as the data container and you have a member variable that is a list of paths
There was a problem hiding this comment.
I thought about that, my main reason why I decided to inherit MetaSwapPackage is that it makes the package object function consistently to other packages. For instance, we have plans to create a MetaSwapModel.dump method, which would dump all packages to file. To have a functional MetaSWAP model, we also need to store the paths to files that need to be copied somewhere. So by wrenching this list of strings/paths in a xarray Dataset, I'm quite sure no special-cased logic is required for this specific filecopy package and keeps things stored consistently in NetCDFs, instead of some custom textfile.
| @classmethod | ||
| def from_imod5_data(cls, imod5_data: Imod5DataDict): | ||
| paths = cast(list[list[str]], imod5_data["extra"]["paths"]) | ||
| paths_unpacked = {Path(p[0]) for p in paths} |
There was a problem hiding this comment.
Why are you using curly brackets and not block brackets? I thought curly brackets are only for dictionaries, but this seems to be a list.
The same thing occurs on line 39
There was a problem hiding this comment.
This is a Python builtin type set. Sets are very useful to check values in iterables for uniqueness. And if one set contains values not present, or present, in another set. For docs, see: https://docs.python.org/3/library/stdtypes.html#set
I wanted to make this more explicit by doing set([Path(p[0]) for p in paths]), but Ruff disagreed with me. I guess the Ruff devs think the set is common knowledge as it is a Python builtin (though a bit more niche).
|
JoerivanEngelen
merged commit 29886af
into
issue_#1260_from_imod5_data_metaswap
Fixes #1320
Description
The iMOD5 projectfile contains an "EXTRA" section, with files to copy. These files usually contain settings and lookup tables for MetaSWAP, which have no direct relation to cell data.
This PR adds CopyFiles package to copy these files.
Checklist
Issue #nr, e.g.Issue #737