array.array initializer does not properly handle memoryviews (and other buffer-protocol objects)

[rhpvorderman]

Bug report

The array.array docs state for the initializer:

If given a list or string, the initializer is passed to the new array’s fromlist(), frombytes(), or fromunicode() method (see below) to add initial items to the array. Otherwise, the iterable initializer is passed to the extend() method.

code:

import array
start = array.array("Q", [1, 2, 3])
untyped_buf = memoryview(start).cast("B")  # Unsigned bytes is the default buffer type IIRC
result = array.array("Q", untyped_buf)

expected = array("Q", [1, 2, 3])
actual = array('Q', [1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0])

What happens is that the memoryview is not treated as a buffer and passed to the from_bytes method. Instead it is treated as a generic iterable.

I discovered this bug when working on a C-extension where I expose almost 7_000_000 64-bit integers to Python using PyMemoryView_FromMemory. Since this is roughly 56 MiB it should be a breeze to memcpy this into an array.array. Instead I get 56 million unsigned bytes types through a Python iterator, which causes a massive bug in my application while also being incredibly slow.

Looking at the code the arraymodule checks for array, bytes, and bytesarray before delegating to the from_bytes method. I think it is much more appropriate to use PyBuffer_Check this will also work appropiately whilst also catching memoryview.

Linked PRs

• gh-101071 Memoryviews properly delegated from array_array_new to array_from_bytes #101073

[rhpvorderman]

Some further discussion points:

• I assumed that memoryview would be properly passed to array_from_bytes. It does not. However, might there be people who rely on this behaviour i.e. array.array("Q", memoryview(data).cast("H")); storing 16 bit unsigned integers in an array with larger values?
• Should other buffer supporting types be treated the same? So a PyBuffer_Check should be used? Or might that be undesirable given that also includes array.array itself and its case is handled specially?

[serhiy-storchaka]

You correctly noticed that it would make the behavior for array and memoryview incompatible. This is a large no for such change. I think that the following changes are possible.

1. array from array copies item by item, and I think that it should support the same behavior for all other buffer supporting types, even if they are not iterable.
2. Support buffer protocol in array.frombytes(). It should just copy underlying bytes. This should help in your use case.

[rhpvorderman]

I simply use array.frombytes explicitly now. I think the array.array constructor does the least surprising thing. Unless you are of course a nerd hand-coding C-extensions without cython and know everything about the underlying data types and the buffer protocol.

I see now that I was surprised because I came from that angle. The current default of always taking the __iter__ protocol does make sense from a consistency standpoint. So I will close this "bug" and the associated PR.

[serhiy-storchaka]

On other hand, the constructor is not consistent now for bytes and bytesarray. It does not treat them the same way as iterable. Oh.

[rhpvorderman]

That is true, but bytes and bytesarray are raw data only. They do not contain typed values in the way arrays and memoryviews contain them. Also it is not unexpected that the initializer passes bytes to frombytes.

I mean the interface could have been designed differently based on this discussion. But this module has been around for years and this introduces a behavior change in the API. That is simply not possible, unless a DeprecationWarning is introduced "warning memoryview will be passed to the frombytes constructor from Python 3.x" onwards. It seems a huge hassle while not solving any problem that cannot currently be solved by using frombytes directly.

[serhiy-storchaka]

The documentation is outdated. I created #114117 to fix it.

[rhpvorderman]

Ah, so I was led astray by the documentation! Bytes-like object indeed suggests that it would work as I expected! So technically the code was wrong, but changing the documentation will not ruin someone's day with unexpected errors later on ;-). Thank you for the discussion!

[serhiy-storchaka]

Thank you for your report. Perhaps we will change some details of implementation, but I do not know how much.