Automatically name index #1974
Automatically name index #1974
Conversation
python/ribasim/ribasim/input_base.py
Outdated
There was a problem hiding this comment.
These are no longer needed because the parser will rename it for us.
python/ribasim/ribasim/schemas.py
Outdated
| @pa.dataframe_parser | ||
| def _name_index(cls, df): | ||
| # Node and Edge have different index names, avoid running both parsers | ||
| if cls.__name__ not in ("NodeSchema", "EdgeSchema"): | ||
| df.index.name = "fid" | ||
| return df |
There was a problem hiding this comment.
I was confused for a while since I expected the methods in the subclasses in edge.py and node.py to override this. But when debugging a test failure I saw that first the node parser was applied, naming it "node_id", and then this one, making it end up with the wrong name. Hence the check for those class names.
There was a problem hiding this comment.
How about df.index.name = cls._index_name()? A mix of both old and new. Needing to check specific schemas in the generic implementation is bad.
There was a problem hiding this comment.
Yeah we could go with that as well.
python/ribasim/ribasim/schemas.py
Outdated
| @@ -30,595 +33,755 @@ def migrate(cls, df: Any, schema_version: int) -> Any: | |||
|
|
|||
| class BasinConcentrationExternalSchema(_BaseSchema): | |||
| fid: Index[Int32] = pa.Field(default=1, check_name=True, coerce=True) | |||
|
|
|||
There was a problem hiding this comment.
gen_python.jl creates even more whitespace, of which ruff now seems to leave one. I tried to fix this with OteraEngine settings but didn't manage. Not so important though.
There was a problem hiding this comment.
Hmm the codegen CI doesn't generate the whitespace, so I removed it again. Not sure why I get different results.
python/ribasim/ribasim/schemas.py
Outdated
| @pa.dataframe_parser | ||
| def _name_index(cls, df): | ||
| # Node and Edge have different index names, avoid running both parsers | ||
| if cls.__name__ not in ("NodeSchema", "EdgeSchema"): | ||
| df.index.name = "fid" | ||
| return df |
There was a problem hiding this comment.
How about df.index.name = cls._index_name()? A mix of both old and new. Needing to check specific schemas in the generic implementation is bad.
Not that I'm aware. I don't understand exactly what kind of debug statement you had in mind. |
Then this PR is really for external users that are playing fast and loose with their dataframes right (not using the add API)? pseudo for debug statement: if df.index.name != cls.index_name():
logging.debug("How dare you")
df.index.name = cls.index_name() |
|
Hmm in this PR I actually removed two explicit index namings because we can now rely on this. I don't think a debug statement then makes sense if we say we'll take care of it. Here you can see a bunch of uses: https://github.com/search?q=repo%3ADeltares%2FRibasim-NL+index.name+%3D&type=code |
Fixes #1968
We let pandera check the index name with
check_name = True. This adds a pandera parser called_name_indexto set the index to the desired name before validating. This still keeps the index names, but offers more user convenience, since the index name is lost with many pandas operations.