feat: add topmodel init_config models
#175
Conversation
aaraney
added
ngen.config
|
@hellkite500, there are a few assumptions that ive made here that we might want to change. In either case, this is ready for a first round of review. |
| assert ( | ||
| len(title.encode()) <= 256 | ||
| ), "`title` must be less that 256 bytes in length" | ||
| data["subcat"] = title |
There was a problem hiding this comment.
I would be inclined to relax this to a warning and simply truncate to 255 (to allow \0 in the c string when read by the code there...)
There was a problem hiding this comment.
Yeah, I get what you mean. Lets meet in the middle and error if its >255. I would rather fail here if someone is using this for validation purposes and they are being really pedantic about the meaning of validation (never been me before 👀).
| data["lnaotb"] = float(distarea_lnaotb[1]) | ||
|
|
||
| num_channels_str = reader.readline() | ||
| assert distarea_lnaotb_str != "", "missing `num_channels`" |
There was a problem hiding this comment.
Wrong variable in check here, should be num channels
| @validator("stand_alone", pre=True) | ||
| @classmethod | ||
| def _coerce_stand_along(cls, value: str | int): | ||
| return int(value) |
There was a problem hiding this comment.
Can we warn if NGEN configs are marked with the standalone flag?
There was a problem hiding this comment.
Yeah, I think that's fair. I'd left it this way originally as I was implementing the bare minimum parser for NextGen. I'll go back and add the functionality to support more complex configuration files.
I think it's probably worth maintaining the complexity of a more complex parser and model for this module in particular. I envision TOPMODEL might be a candidate bmi module for a layered configuration in the future. What are your thoughts, @hellkite500?
There was a problem hiding this comment.
I generally agree. And I think if we are building ngen specific configuration and validation, we should at least provide warnings of incompatible instances when we can.
There was a problem hiding this comment.
Just pushed up this feature.
There was a problem hiding this comment.
Ooooops, you right. I put a warning somewhere else but not here. Just added it. My b.
Initial structures for parsing
topmodelconfiguration files. Still a few things to sort so it is more compliant withngen.init_config, but this is a good start.