Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3] New Mypy errors now on v3 branch #2131

Closed
jhamman opened this issue Aug 28, 2024 · 3 comments · Fixed by #2139
Closed

[v3] New Mypy errors now on v3 branch #2131

jhamman opened this issue Aug 28, 2024 · 3 comments · Fixed by #2139
Labels
help wanted Issue could use help from someone with familiarity on the topic types V3 Affects the v3 branch
Milestone

Comments

@jhamman
Copy link
Member

jhamman commented Aug 28, 2024

We've started seeing the following errors in our pre-commit action. Would be good if we can give these some attention this week.

src/zarr/core/metadata.py:586: error: Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader  [misc]
src/zarr/core/metadata.py:586: error: Variable "zarr.core.metadata.INTEGER_DTYPE" is not valid as a type  [valid-type]
src/zarr/core/metadata.py:586: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
src/zarr/core/metadata.py:598: error: Variable "zarr.core.metadata.INTEGER_DTYPE" is not valid as a type  [valid-type]
src/zarr/core/metadata.py:598: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
src/zarr/core/metadata.py:621: error: Item INTEGER_DTYPE? of "BoolDType | INTEGER_DTYPE? | Float16DType | Float32DType | Float64DType | Complex64DType | Complex128DType" has no attribute "type"  [union-attr]
src/zarr/core/metadata.py:637: error: Item INTEGER_DTYPE? of "BoolDType | INTEGER_DTYPE? | Float16DType | Float32DType | Float64DType | Complex64DType | Complex128DType" has no attribute "type"  [union-attr]
src/zarr/codecs/crc32c_.py:40: error: Argument 1 to "crc32c" has incompatible type "ndarray[Any, dtype[Any]]"; expected "Buffer"  [arg-type]
src/zarr/codecs/crc32c_.py:55: error: Argument 1 to "crc32c" has incompatible type "ndarray[Any, dtype[Any]]"; expected "Buffer"  [arg-type]
src/zarr/codecs/sharding.py:104: error: Returning Any from function declared to return "tuple[int, ...]"  [no-any-return]
Found 8 errors in 3 files (checked 66 source files)
@jhamman jhamman added V3 Affects the v3 branch types labels Aug 28, 2024
@jhamman jhamman added this to the 3.0.0 milestone Aug 28, 2024
@jhamman jhamman added the help wanted Issue could use help from someone with familiarity on the topic label Aug 28, 2024
@TomAugspurger
Copy link
Contributor

I think that numpy/numpy#27211 is the root cause of src/zarr/codecs/sharding.py:104: error: Returning Any from function declared to return "tuple[int, ...]" [no-any-return].

I think we'll need to work around it for now.

@TomAugspurger
Copy link
Contributor

I'm seeing a slightly different error for the dtype stuff in core.metadata:

src/zarr/core/metadata.py:189: error: No overload variant of "parse_fill_value_v3" matches argument types "Any", "dtype[Any]"  [call-overload]
src/zarr/core/metadata.py:189: note: Possible overload variants:
src/zarr/core/metadata.py:189: note:     def parse_fill_value_v3(fill_value: Any, dtype: BoolDType) -> bool
src/zarr/core/metadata.py:189: note:     def parse_fill_value_v3(fill_value: Any, dtype: Int8DType | Int16DType | Int32DType | Int64DType | UInt8DType | UInt16DType | UInt32DType | UInt64DType) -> signedinteger[_8Bit] | signedinteger[_16Bit] | signedinteger[_32Bit] | signedinteger[_64Bit] | unsignedinteger[_8Bit] | unsignedinteger[_16Bit] | unsignedinteger[_32Bit] | unsignedinteger[_64Bit]
src/zarr/core/metadata.py:189: note:     def parse_fill_value_v3(fill_value: Any, dtype: Float16DType | Float32DType | Float64DType) -> floating[_16Bit] | floating[_32Bit] | floating[_64Bit]
src/zarr/core/metadata.py:189: note:     def parse_fill_value_v3(fill_value: Any, dtype: Complex64DType | Complex128DType) -> complexfloating[_32Bit, _32Bit] | complexfloating[_64Bit, _64Bit]

I think this is coming down to a difference in static typing between a dtype declared as np.types.<dtype> and np.dtype(<dtype>). Given a file like

import numpy as np
from zarr.core.metadata import parse_fill_value_v3


reveal_type(parse_fill_value_v3(1, np.dtypes.UInt64DType()))
reveal_type(parse_fill_value_v3(1, np.dtype("uint64")))

mypy shows

foo.py:5: note: Revealed type is "Union[numpy.signedinteger[numpy._typing._8Bit], numpy.signedinteger[numpy._typing._16Bit], numpy.signedinteger[numpy._typing._32Bit], numpy.signedinteger[numpy._typing._64Bit], numpy.unsignedinteger[numpy._typing._8Bit], numpy.unsignedinteger[numpy._typing._16Bit], numpy.unsignedinteger[numpy._typing._32Bit], numpy.unsignedinteger[numpy._typing._64Bit]]"
foo.py:6: error: No overload variant of "parse_fill_value_v3" matches argument types "int", "dtype[unsignedinteger[_64Bit]]"  [call-overload]
foo.py:6: note: Possible overload variants:
foo.py:6: note:     def parse_fill_value_v3(fill_value: Any, dtype: BoolDType) -> bool
foo.py:6: note:     def parse_fill_value_v3(fill_value: Any, dtype: Int8DType | Int16DType | Int32DType | Int64DType | UInt8DType | UInt16DType | UInt32DType | UInt64DType) -> signedinteger[_8Bit] | signedinteger[_16Bit] | signedinteger[_32Bit] | signedinteger[_64Bit] | unsignedinteger[_8Bit] | unsignedinteger[_16Bit] | unsignedinteger[_32Bit] | unsignedinteger[_64Bit]
foo.py:6: note:     def parse_fill_value_v3(fill_value: Any, dtype: Float16DType | Float32DType | Float64DType) -> floating[_16Bit] | floating[_32Bit] | floating[_64Bit]
foo.py:6: note:     def parse_fill_value_v3(fill_value: Any, dtype: Complex64DType | Complex128DType) -> complexfloating[_32Bit, _32Bit] | complexfloating[_64Bit, _64Bit]
foo.py:6: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)

I'm not sure yet whether this is expected.

For now, we can work around this with

@overload
def parse_fill_value_v3(fill_value: Any, dtype: np.dtype[Any]) -> Any: ...

I'm not 100% sure, but I think this is no worse than we were before.

@jhamman
Copy link
Member Author

jhamman commented Aug 30, 2024

Thanks @dstansby and @TomAugspurger for sorting these out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue could use help from someone with familiarity on the topic types V3 Affects the v3 branch
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants