Title: memoryview: test slice clamping
Type: Stage: resolved
Components: Versions:
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: skrah, terry.reedy, vstinner
Priority: normal Keywords: patch

Created on 2014-06-24 09:19 by vstinner, last changed 2014-06-28 15:33 by terry.reedy. This issue is now closed.

File name Uploaded Description Edit
memoryview_test_large_slice.patch vstinner, 2014-06-27 20:15 review
Messages (9)
msg221441 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-24 09:19
As I following up to the issue #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.
msg221443 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-06-24 09:45
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)
msg221477 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-06-24 18:20
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."
msg221711 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-27 19:10
#21831 was about size not being properly clamped. Here it is.
msg221717 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-06-27 20:15
If the behaviour is well expected, I suggest to add an unit test: memoryview_test_large_slice.patch.
msg221724 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-27 20:46
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)
msg221780 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-06-28 12:14
Since the rewrite in 3.3 many memoryview tests are actually in
msg221782 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-06-28 12:38
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.
msg221797 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-28 15:33
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 #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.
Date User Action Args
2014-06-28 15:33:05terry.reedysetstatus: open -> closed
resolution: not a bug
messages: + msg221797
2014-06-28 15:11:37pitrousettitle: memoryview: test slick clamping -> memoryview: test slice clamping
2014-06-28 12:38:47skrahsetmessages: + msg221782
2014-06-28 12:14:40skrahsetmessages: + msg221780
2014-06-27 20:46:27terry.reedysetmessages: + msg221724
title: memoryview: no overflow on large slice values (start, stop, step) -> memoryview: test slick clamping
2014-06-27 20:15:54vstinnersetstatus: closed -> open
files: + memoryview_test_large_slice.patch
messages: + msg221717

keywords: + patch
resolution: not a bug -> (no value)
2014-06-27 19:10:11terry.reedysetstatus: open -> closed

nosy: + terry.reedy
messages: + msg221711

resolution: not a bug
stage: resolved
2014-06-24 18:20:15skrahsetmessages: + msg221477
2014-06-24 09:45:16skrahsetmessages: + msg221443
2014-06-24 09:19:07vstinnercreate