More precise type hints for slice constructor#12899
More precise type hints for slice constructor#12899randolf-scholz wants to merge 11 commits intopython:mainfrom
slice constructor#12899Conversation
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.
AlexWaygood
left a comment
There was a problem hiding this comment.
Impressive! There's some opportunities for simplification here.
This is complex enough that I think we could probably do with some test cases in stdlib/@tests/test_cases.
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])