memoryview: test slice clamping

[vstinner]

BPO	21856
Nosy	@terryjreedy, @vstinner, @skrah
Files	memoryview_test_large_slice.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-06-28.15:33:05.937>
created_at = <Date 2014-06-24.09:19:07.333>
labels = ['invalid']
title = 'memoryview: test slice clamping'
updated_at = <Date 2014-06-28.15:33:05.936>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2014-06-28.15:33:05.936>
actor = 'terry.reedy'
assignee = 'none'
closed = True
closed_date = <Date 2014-06-28.15:33:05.937>
closer = 'terry.reedy'
components = []
creation = <Date 2014-06-24.09:19:07.333>
creator = 'vstinner'
dependencies = []
files = ['35793']
hgrepos = []
issue_num = 21856
keywords = ['patch']
message_count = 9.0
messages = ['221441', '221443', '221477', '221711', '221717', '221724', '221780', '221782', '221797']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'vstinner', 'skrah']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue21856'
versions = []

[vstinner]

As I following up to the issue bpo-21831, I don't understand why memoryview[2**100:] doesn't raise an overflow error!? It looks like a bug.

Attached patch changes the behaviour to raise an OverflowError.

[skrah]

It's a slice of length zero:

>>> b = bytearray([1,2,3,4])
>>> m = memoryview(b)
>>>
>>> b2 = b[2**100:]
>>> m2 = m[2**100:]
>>>
>>> list(b2)
[]
>>> list(m2)
[]
>>>

[skrah]

Victor, shall we close this? The behavior is basically as specified:

"The slice of s from i to j is defined as the sequence of items with index k such that i <= k < j. If i or j is greater than len(s), use len(s). If i is omitted or None, use 0. If j is omitted or None, use len(s). If i is greater than or equal to j, the slice is empty."

[terryjreedy]

bpo-21831 was about size not being properly clamped. Here it is.

[vstinner]

If the behaviour is well expected, I suggest to add an unit test: memoryview_test_large_slice.patch.

[terryjreedy]

Memoryview should definitely have the same slice tests as other sequence objects. Go ahead and check.

I believe slice clamping itself should now be done by slice.indices, not by each class.

S.indices(len) -> (start, stop, stride)

Assuming a sequence of length len, calculate the start and stop
indices, and the stride length of the extended slice described by
S. Out of bounds indices are clipped in a manner consistent with the
handling of normal slices.

It seems like this was written before it was normal for every slice to have a step (stride), defaulting to None/1. It definitely comes from 2.x, before __getslice__ was folded into __getitem__

I expect builtin 3.x sequence objects should all use something like the C equivalent of
def __getitem__(self, ob):
if type(ob) is slice:
start, stop, stride = ob.indices(self.len)
...

[skrah]

Since the rewrite in 3.3 many memoryview tests are actually in test_buffer.py.

[skrah]

And Terry is right, the actual slice clamping happens in
PySlice_GetIndicesEx(), which should always produce values
that are in the correct range. Hence the tests focus on
slices that already are in the correct range.

I'm not sure if PySlice_GetIndicesEx() itself has tests somewhere.

[terryjreedy]

Seaching for "PySlice_GetIndicesEx" in .../test/*.py with Idle Find in Files did not turn up any hits. However, test.test_slice.test_indices() has several tests of clamping, including 2**100 in various combinations. The use of 2**100 was was added in bpo-14794 to test successful removal of the Overflow that Victor requested. I don't see that there is any patch needed. Victor, if you find a deficiency, reopen or open a new issue.