aboderinsamuel commented on code in PR #50203:
URL: https://github.com/apache/arrow/pull/50203#discussion_r3459004281
##########
python/pyarrow/src/arrow/python/python_to_arrow.cc:
##########
@@ -908,8 +908,20 @@ class PyListConverter : public ListConverter<T,
PyConverter, PyConverterTrait> {
Status AppendNdarray(PyObject* value) {
PyArrayObject* ndarray = reinterpret_cast<PyArrayObject*>(value);
+ OwnedRef flattened;
if (PyArray_NDIM(ndarray) != 1) {
- return Status::Invalid("Can only convert 1-dimensional array values");
+ // GH-49644: a fixed-size list (e.g. the storage of a fixed-shape tensor)
+ // can be built from a multi-dimensional array by flattening it in C
+ // order. The total number of elements must still match the list size,
+ // which the builder validates below. 0-dimensional arrays and
+ // variable-sized lists remain restricted to 1-dimensional values.
+ if (PyArray_NDIM(ndarray) < 2 || this->list_type_->id() !=
Type::FIXED_SIZE_LIST) {
+ return Status::Invalid("Can only convert 1-dimensional array values");
+ }
+ flattened.reset(PyArray_Ravel(ndarray, NPY_CORDER));
Review Comment:
PyArray_Ravel(ndarray, NPY_CORDER) returns a 1-D, C-order view, which lets
me reuse the existing 1-D append paths unchanged (the typed fast paths and the
PySequence fallback). It's zero-copy when the input is already C-contiguous and
only copies for non-contiguous/Fortran-order input, where a copy is unavoidable
to get C-order data anyway, which also matches the fixed-shape-tensor's
row-major storage.
Alternatives: PyArray_Flatten(NPY_CORDER) gives the same result but always
copies; a manual NpyIter/buffer loop could avoid even the view allocation but
would mean duplicating the typed-append fast paths. Ravel was the smallest
change with the best reuse/performance balance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
