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

Disallow literal 0 step in slice expressions #18065

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

brianschubert
Copy link
Collaborator

@brianschubert brianschubert commented Oct 28, 2024

Follow up to #18063

Using a step of 0 when slicing a sequence is often a runtime error:

>>> ()[::0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: slice step cannot be zero

This is true for standard library sequence types (tuple, list, str, memoryview, array.array, etc.), as well as for numpy arrays, pandas series, and many other third-party sequence types (including all that use slice.indices internally).

Strictly speaking, creating a slice with a step of zero is possible, similar to how creating a slice with non-integer-like start/stop/step values is possible. However, just like non-integer start/stop/step values, this is usually a mistake (and one that will cause a runtime error) as opposed to an intended API. Mypy currently warns about non-integer start/stop/step values, so also warning about using a step of 0 seems consistent.

This PR is fairly liberal in what it will warn about: it disallows any expressions with a literal-0 type (or unions including a literal 0) in the step position of a slice expression. Depending on how significant the mypy_primer hit is, I may relax this a bit.

This comment has been minimized.

@brianschubert
Copy link
Collaborator Author

mypy_primer hits look good. All hits are from tests that assert that using a step of 0 raises at runtime.

Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
+ pandas/tests/series/accessors/test_list_accessor.py:100: error: Slice step cannot be 0  [misc]
+ pandas/tests/series/accessors/test_list_accessor.py:103: error: Slice step cannot be 0  [misc]
+ pandas/tests/series/accessors/test_list_accessor.py:118: error: Slice step cannot be 0  [misc]
+ pandas/tests/indexing/test_indexing.py:716: error: Slice step cannot be 0  [misc]

sympy (https://github.com/sympy/sympy)
+ sympy/sets/tests/test_fancysets.py:273: error: Slice step cannot be 0  [misc]
+ sympy/sets/tests/test_fancysets.py:279: error: Slice step cannot be 0  [misc]

@brianschubert
Copy link
Collaborator Author

Thinking about this more, we probably want to move mypy away from this sort of hardcoded (and lint-y) checks. Now that slices are generic, I imagine mypy's handling of slice expressions will be seeing other improvements soon, which may supersede this. Marking this as a draft until those changes settle

@brianschubert brianschubert marked this pull request as draft November 18, 2024 02:35
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.

1 participant