Fix mypy type errors with xarray DataVariables #547
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes type checking errors with xarray DataVariables in newer mypy versions.
Cast Hashable keys to str to satisfy mypy's stricter type checking with xarray DataVariables in Python 3.13.
ecomodeller
changed the title
Updated parsing utility functions to accept Sequence[Hashable] rather than Sequence[str], which properly handles xarray's DataVariables type while maintaining compatibility with DataFrame integer indices. - Changed _get_name() and _get_idx() to accept Sequence[Hashable] - Used cast(str, ...) to satisfy type checker while preserving actual types - Updated all parsing functions to accept Hashable sequences - Kept explicit str() conversions only where needed (e.g., dict keys)
Added 'valid_items: Sequence[Hashable]' type annotations to help mypy understand the correct type when creating lists from data_vars.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves mypy type checking errors related to xarray DataVariables type inference.
Changes:
Sequence[Hashable]src/modelskill/utils.py(_get_name,_get_idx)src/modelskill/timeseries/_point.py(_parse_point_items)src/modelskill/timeseries/_track.py(_parse_track_items)src/modelskill/model/_base.py(SelectedItems.parse,_parse_items)src/modelskill/comparison/_comparison.py(direct xarray access)Background: str vs Hashable
xarray's type hints specify that
Dataset.data_varsreturns an iterator ofHashablekeys, because the xarray API technically allows any hashable type as a variable name. However, in practice, variable names are always strings.When mypy 1.19.1 type-checks code passing these variable names to functions expecting
Sequence[str], it correctly flags the type mismatch betweenHashableandstr.Solution: Fix type hints at the root
Rather than converting
Hashabletostrat call sites (redundant), we updated the parsing utility functions to acceptSequence[Hashable]. This required:_get_name()and_get_idx()to acceptSequence[Hashable]cast(str, ...)in_get_name()to satisfy mypy while preserving actual types (important for DataFrame integer indices)_parse_point_items,_parse_track_items,SelectedItems.parse) to acceptSequence[Hashable]str()conversions only where needed (e.g., dict keys in_comparison.py)This approach fixes the type system at its root while maintaining runtime compatibility with both string variable names (xarray) and integer column indices (DataFrame RangeIndex).