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

[python] Make Slice protocol not runtime_checkable #128

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

thetorpedodog
Copy link
Contributor

Since range has the members start/stop/step, that meant that isinstance(range(x), Slice) would return True, when a range is not a Slice. Using abc and manually registering slice fixes this.

This also unrestricts Slices from being Comparable, since the built-in slice type does not have this restriction, and adds the is_slice_of function, taken from tiledbsoma.

@thetorpedodog thetorpedodog changed the title Implement Slice protocol as abc, not Protocol. [python] Make Slice protocol not runtime_checkable. Feb 15, 2023
@johnkerl johnkerl changed the title [python] Make Slice protocol not runtime_checkable. [python] Make Slice protocol not runtime_checkable Feb 15, 2023
@thetorpedodog
Copy link
Contributor Author

Update: Since mypy doesn’t currently understand SomeABC.register(other_type), that approach Did Not Work well in reality. Instead, I’ve just removed the runtime_checkableility of the protocol and made is_slice_of the canonical type-checking method.

Since `range` has the members `start`/`stop`/`step`, that meant that
`isinstance(range(x), Slice)` would return True, when a `range` is *not*
a `Slice`.  Instead, we only provide the `is_slice_of` function
(originally from `tiledbsoma`) to perform the appropriate type check.

This also unrestricts `Slice`s from being `Comparable`, since the
built-in `slice` type does not have this restriction.
@thetorpedodog
Copy link
Contributor Author

Oops, had not noticed the approval. Reverting to the approved state (i.e. no longer removing Comparable) and saving that for another change.

@thetorpedodog thetorpedodog merged commit 1482d48 into main Feb 15, 2023
@thetorpedodog thetorpedodog deleted the overbroad-slice-protocol branch February 15, 2023 20:01
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.

2 participants