The call to PyBuffer_IsContiguous() (see patch) is redundant:

PyBUF_WRITABLE is a flag that can be added to any buffer request.
The real request here is (PyBUF_SIMPLE|PyBUF_WRITABLE), which is
equal to PyBUF_WRITABLE since PyBUF_SIMPLE==0.

PyBUF_SIMPLE implies C-contiguous with format 'B'.


Perhaps the check was added for broken buffer providers, but I
think at some point we should assume correct providers.

Do you have unit test with non contiguous buffers? If not, it would help to have such buffer in _testcapi.

I think memoryview(bytearray)[::2] provides non contiguous buffers. But I'm not sure this is tested.

Yes, _testbuffer.ndarray can create any buffer.

A unit test would not be so helpful. The potential problem is that
third party extensions with broken getbufferprocs would suffer.


But at some point we have to streamline PEP-3118 code, or it
will remain inscrutable forever.  Extension writers often copy
code from cpython, and I'm afraid if we don't remove redundancy,
people will think such contiguity checks are necessary.

> Yes, _testbuffer.ndarray can create any buffer.

Cool. So could you please add non regression tests to test_w_star() of test_getargs2?

Other formats expect a contiguous buffer: 'y*', 's*', 'z*'. 
Formats which "convert" a buffer: 'y', 's#', 'z#.

> A unit test would not be so helpful. The potential problem is that
> third party extensions with broken getbufferprocs would suffer.

I don't understand the link between third party extensions and
test_getargs2. test_getargs2 is a unit test for non-regression of
CPython. Can you please elaborate?

STINNER Victor wrote:
> I don't understand the link between third party extensions and
> test_getargs2. test_getargs2 is a unit test for non-regression of
> CPython. Can you please elaborate?

A test failure needs a broken buffer provider that hands out a non-contiguous
buffer in response to a PyBUF_SIMPLE request.

The only non-contiguous buffer provider in the stdlib is memoryview.
If I break memoryview's getbufferproc ...

diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c
--- a/Objects/memoryobject.c
+++ b/Objects/memoryobject.c
@@ -1465,11 +1465,6 @@
         return -1;
     }
     if (!REQ_STRIDES(flags)) {
-        if (!MV_C_CONTIGUOUS(baseflags)) {
-            PyErr_SetString(PyExc_BufferError,
-                "memoryview: underlying buffer is not C-contiguous");
-            return -1;
-        }
         view->strides = NULL;
     }
     if (!REQ_SHAPE(flags)) {

...

test_buffer already fails.  So what should I test?  Perhaps ctypes has
some capabilities that I'm unaware of.

If I'm using _testbuffer.ndarray(), I'm effectively testing that the
getbufferproc of ndarray is correct (which is also tested multiple times
in test_buffer).

Well, here's a patch with tests.

Victor, I think you added the contiguity test in 9d49b744078c. Do you
remember why?

> Victor, I think you added the contiguity test in 9d49b744078c. Do you
remember why?

I don't understand the change like that. The call to PyBuffer_IsContiguous(view, 'C') was older than this changeset.

Ah yes, it seems to originate from #3139.

See also contiguity tests in Modules/binascii.c and Modules/_ssl.c,

messages: + msg235289

nosy: + larry
messages: + msg235244

keywords: + patch