More precise type hints for slice constructor
#12899
Conversation
This comment has been minimized.
This comment has been minimized.
|
Please do the positional-only fix in its own PR and I will instantly merge it. Then we can evaluate the other changes you'd like to make in isolation and consider what the costs and benefits might be. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This line in pandas raises an important issue: we likely want elif isinstance(loc, slice):
step = loc.step if loc.step is not None else 1
inds.extend(range(loc.start, loc.stop, step))However, range actually does not support This issue potentially makes it more sensible to, instead of making |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
|
@AlexWaygood One thing I am not super sure if I am happy with is that: s = slice(None, 2)
reveal_type(s) # `slice[int | None, int | None, Any]` with current overloads
reveal_type(s.stop) # `int | None` with current overloadsBut I think a lot of users would expect that I created #12904 to check if it has any difference on mypy primer. |
You can also experiment with |
|
Diff from mypy_primer, showing the effect of this PR on open source code: xarray (https://github.com/pydata/xarray)
+ xarray/tests/test_backends.py: note: In member "test_write_region_mode" of class "ZarrBase":
+ xarray/tests/test_backends.py:3019: error: No overload variant of "to_zarr" of "Dataset" matches argument types "Any", "object", "Any", "dict[str, Any]" [call-overload]
+ xarray/tests/test_backends.py:3019: note: Possible overload variants:
+ xarray/tests/test_backends.py:3019: note: def to_zarr(self, store: MutableMapping[Any, Any] | str | PathLike[str] | None = ..., chunk_store: MutableMapping[Any, Any] | str | PathLike[Any] | None = ..., mode: Literal['w', 'w-', 'a', 'a-', 'r+', 'r'] | None = ..., synchronizer: Any = ..., group: str | None = ..., encoding: Mapping[Any, Any] | None = ..., *, compute: Literal[True] = ..., consolidated: bool | None = ..., append_dim: Hashable | None = ..., region: Mapping[str, slice[Any, Any, Any] | Literal['auto']] | Literal['auto'] | None = ..., safe_chunks: bool = ..., storage_options: dict[str, str] | None = ..., zarr_version: int | None = ..., zarr_format: int | None = ..., write_empty_chunks: bool | None = ..., chunkmanager_store_kwargs: dict[str, Any] | None = ...) -> ZarrStore
+ xarray/tests/test_backends.py:3019: note: def to_zarr(self, store: MutableMapping[Any, Any] | str | PathLike[str] | None = ..., chunk_store: MutableMapping[Any, Any] | str | PathLike[Any] | None = ..., mode: Literal['w', 'w-', 'a', 'a-', 'r+', 'r'] | None = ..., synchronizer: Any = ..., group: str | None = ..., encoding: Mapping[Any, Any] | None = ..., *, compute: Literal[False], consolidated: bool | None = ..., append_dim: Hashable | None = ..., region: Mapping[str, slice[Any, Any, Any] | Literal['auto']] | Literal['auto'] | None = ..., safe_chunks: bool = ..., storage_options: dict[str, str] | None = ..., zarr_version: int | None = ..., zarr_format: int | None = ..., write_empty_chunks: bool | None = ..., chunkmanager_store_kwargs: dict[str, Any] | None = ...) -> Any
+ xarray/tests/test_backends.py:3019: error: Argument 1 to "isel" of "Dataset" has incompatible type "object"; expected "Mapping[Any, Any] | None" [arg-type]
|
|
Closing in favor of #12904 due to #12899 (comment) |
startandsteptoNoneif not given.Some thoughts:
[slice(None)], then modifies withslice(None, n).slice(None, T)with aslice(T, None).There are 2 coupled questions:
slice,slice[A],slice[A, B]andslice[A, B, C]? This is controlled via thedefaultsof theTypeVar.slice(None),slice(b),slice(a,b)andslice(a, b, c)? This is controlled via the__new__overloads.It seems most common that people want
slice(None, x)andslice(x, None)to be compatible, which makes sense as theNonesimply indicates open-ended slice.Idea: create a complete classification:
sliceshould be compatible with the "all-slices":slice(None),slice(None, None)andslice(None, None, None). (⟿slice[?, ?, ?])slice[T]should be compatible with:slice(None),slice(None, None)andslice(None, None, None)(⟿slice[?, ?, ?])slice(t),slice(None, t)andslice(None, t, None). (⟿slice[?, T, ?])slice(t, None)andslice(t, None, None). (⟿slice[T, ?, ?])slice(t, t)andslice(t, t, None). (⟿slice[T, T, ?])slice[X, Y]should be compatible with:slice(None),slice(None, None)andslice(None, None, None)(⟿slice[?, ?, ?])slice(y),slice(None, y)andslice(None, y, None). (⟿slice[?, Y, ?])slice(x, None)andslice(x, None, None)(⟿slice[X, ?, ?])slice(x, y)andslice(x, y, None). (⟿slice[X, Y, ?])slice[X, Y, Z]should be compatible with:slice(None),slice(None, None)andslice(None, None, None). (⟿slice[?, ?, ?])slice(y),slice(None, y)andslice(None, y, None). (⟿slice[?, Y, ?])slice(x, None)andslice(x, None, None)(⟿slice[X, ?, ?])slice(x, y)andslice(x, y, None). (⟿slice[X, Y, ?])slice(None, None, z)(⟿slice[?, ?, Z])slice(None, y, z)(⟿slice[?, Y, Z])slice(x, None, z)(⟿slice[X, ?, Z])slice(x, y, z)(⟿slice[X, Y, Z])