Enable and improve mypy typechecking #2422
Conversation
VeckoTheGecko
force-pushed
the
enable-mypy
branch
2 times, most recently
from
5bb5967 to
0a263d0
Compare
- remove _discover_U_and_V - move helper function
Allowing for gradual resolution
VeckoTheGecko
force-pushed
the
enable-mypy
branch
from
0a263d0 to
e9bc355
Compare
| def _ds_rename_using_standard_names(ds: xr.Dataset | ux.UxDataset, name_dict: dict[str, str]) -> xr.Dataset: | ||
| for standard_name, rename_to in name_dict.items(): | ||
| name = ds.cf[standard_name].name | ||
| ds = ds.rename({name: rename_to}) | ||
| logger.info( | ||
| f"cf_xarray found variable {name!r} with CF standard name {standard_name!r} in dataset, renamed it to {rename_to!r} for Parcels simulation." | ||
| ) | ||
| return ds | ||
|
|
||
|
|
There was a problem hiding this comment.
Should calling this function not be part of convert?
Once a user requests a FieldSet, do we really want to still change field names? There would be no way to override that for a user anymore.
There was a problem hiding this comment.
I'm only moving these here since its only use is in the ux fieldset ingestion - and that hasn't yet moved to convert. I'm happy either way (I thought having it here with the other functionality was neater, especially since we're probably going to outright drop this functionality due to us not doing U and V discovery in convert.py)
There was a problem hiding this comment.
OK, clear. Then perhaps add a TODO that we will likely drop this function in the near future?
| assert xi is not None | ||
| assert yi is not None |
There was a problem hiding this comment.
For my own understanding; why are these new lines needed?
2 participants
This PR is incremental work fixing typechecking errors in parcels using Parcels. Besides fixing mypy errors, we have the following changes:
CfAxesinparcels._typingI have enabled the typechecking in CI - it isn't passing yet, but I will be working in future PRs to reduce the number of errors. This workflow can also be run using
pixi run typing.