FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

`builtins.slice`: more precise `__new__` overloads and let `StopT` default to `StartT`. by randolf-scholz · Pull Request #12904 · python/typeshed · GitHub

Repository navigation

builtins.slice: more precise __new__ overloads and let StopT default to StartT. - #12904

Closed
randolf-scholz wants to merge 32 commits into
python:mainfrom
randolf-scholz:slice_generic_alternative
Closed

randolf-scholz wants to merge 32 commits into
python:mainfrom
randolf-scholz:slice_generic_alternative

Conversation

randolf-scholz commented Oct 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Different overloads than #12899, testing mypy primer.

  • added check_slice.py stubtest, testing both slice.__new__ constructor and slice[T] annotation
  • changed StopT to default to StartT instead of Any. For example:
    • slice[int] = slice[int, int, Any] represents finite intervals
    • slice[int | None] = slice[int | None, int | None, Any] arbitrary intervals
  • Added more comprehensive suite of overloads for __new__, removed special casing for int types.

slice.__new__ now follows a very simply rule: any None argument will result in the corresponding Generic Type to be considered as Any.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

randolf-scholz marked this pull request as ready for review October 25, 2024 12:25
randolf-scholz changed the title [test] Slice generic alternative builtins.slice: more precise __new__ overloads and let StopT default to StartT. Oct 25, 2024
randolf-scholz marked this pull request as draft October 25, 2024 12:31

AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks promising!

Comment thread stdlib/builtins.pyi
randolf-scholz marked this pull request as ready for review October 25, 2024 12:35

This comment has been minimized.

AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nice! Looks like you now just need to update some assertions in the test-case file to use Union[int, Any] rather than Any in order to make the test case pass.

Comment thread stdlib/builtins.pyi
Comment thread stdlib/builtins.pyi
randolf-scholz and others added 4 commits October 25, 2024 15:32
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

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>

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Copy Markdown
Contributor Author

One doubt I have is whether we should change it so that slice[A] = slice[A, A, A] after all.

Without this convention, describing the most commonly used slice type, which I think is

slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None]

Probably requires an alias since it is so long.

Moreover, I think we need to clarify whether the collections.abc.Sequence protocol requires __getitem__ to support slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], or if slice[int | None, int | None, int | None] is sufficient. This is likely a critical case as many users want to subclass Sequence, and it shouldn't be too inconvenient to type hint the implementation.

randolf-scholz commented Nov 13, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@AlexWaygood #13008 is the same as this, but defaults step to start | stop, so that slice[SupportsIndex | None] gives us the most commonly used slice type. In this case, people who want to use datetime/timedelta need to annotate with 3 values.

A decision needs to be made between:

  1. Use this patch. For type hinting index-slices there are 3 options:
    1. Guide users to type hint slice[SupportsIndex | None], which means step will not be checked.
    2. Guide users to use an IndexSlice type alias provided by typing_extensions / typing.
    3. Guide users to use their own alias.
  2. Use builtins.slice: more precise __new__ overloads and defaults for StopT and StepT. #13008, and guide users to annotate index-slices with slice[SupportsIndex | None], and users who want to use non-standard slices to provide all 3 type variables.

This comment has been minimized.

github-actions Bot commented Jan 6, 2025

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

srittau commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

Closing in favor of #13008.

srittau closed this Feb 28, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL