Skip to content

Conversation

max-sixty
Copy link
Collaborator

@max-sixty max-sixty commented Sep 18, 2025

From Claude:


Summary

This PR removes xarray.tests.test_dataarray from the mypy exclusions list, enabling type checking for this test file.

Changes Made

  • Removed test_dataarray from the mypy check_untyped_defs = false exclusion list in pyproject.toml
  • Fixed all resulting type errors with minimal, targeted fixes:
    • Added type: ignore comments for test cases that intentionally test error conditions
    • Added type: ignore for the scipy import fallback pattern
    • Split test_astype_attrs to handle Variable separately from DataArray/Dataset due to different method signatures
    • Added type: ignore for MultiIndex.get_level_values calls where level names could theoretically be None

Test Plan

  • All tests in test_dataarray.py continue to pass
  • dmypy run shows no type errors
  • Pre-commit hooks pass

This is part of the ongoing effort to improve type coverage in the xarray test suite.

This commit removes test_dataarray from the mypy exclusions list and fixes
all resulting type errors:

- Add type ignores for intentional test cases that verify error handling
- Add type ignore for scipy import fallback to object
- Split test_astype_attrs to handle Variable separately from DataArray/Dataset
  due to different method signatures (Variable.astype doesn't have keep_attrs)
- Add type ignores for MultiIndex.get_level_values calls where names may be None

All tests continue to pass after these changes.

Co-authored-by: Claude <noreply@anthropic.com>
@max-sixty max-sixty force-pushed the mypy-reduce-exclusion-da branch from a50f247 to 5ba8d2a Compare September 18, 2025 03:34
This email format is more likely to be recognized by GitHub for bot/AI
co-authorship attribution.

Co-authored-by: Claude <noreply@anthropic.com>
@max-sixty max-sixty added the plan to merge Final call for comments label Sep 18, 2025
Use the established pattern of including unused-ignore in the type ignore
comments for cases where the ignore necessity varies by environment:
- scipy_.py: scipy import is optional, so ignore is unused when scipy is installed
- test_dataarray.py: pandas-stubs versions vary in their MultiIndex.names typing

Co-authored-by: Claude <noreply@anthropic.com>
Comment on lines 2343 to 2347
# Variable has different astype signature without keep_attrs parameter
va = self.va.copy()
va.attrs["foo"] = "bar"
assert va.attrs == va.astype(float).attrs
assert not va.astype(float, keep_attrs=False).attrs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert, no code difference. But maybe look in to Variable.astype?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to clarify the comment

The split is required for mypy type checking, not due to API differences.
Variable, DataArray, and Dataset don't share a common base class that mypy
recognizes, so when they're in the same list, mypy infers the type as
'object', which lacks the attrs and astype methods.

Co-authored-by: Claude <noreply@anthropic.com>
@max-sixty max-sixty merged commit 7228e8f into pydata:main Sep 18, 2025
37 checks passed
@max-sixty max-sixty deleted the mypy-reduce-exclusion-da branch September 18, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants