Skip to content

Conversation

@jorisvandenbossche
Copy link
Member

https://issues.apache.org/jira/browse/ARROW-5790

In the NumPyConverter constructor, we access the strides of the array with PyArray_STRIDES(arr_)[0], so need to ensure that the array is 1D before passing it there.

return Status::Invalid("Input object was not a NumPy array");
}
if (PyArray_NDIM(reinterpret_cast<PyArrayObject*>(ao)) != 1) {
return Status::Invalid("only handle 1-dimensional arrays");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NumPyConverter::Convert does the check again, but sadly we cannot raise from the constructor, so we'd need a factory to return the Status...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that check can probably be removed? (as the NumpyConverter is only used from this function)

@kszucs kszucs changed the title ARROW-5790: [Python] raise error when trying to convert 0-dim array in pa.array ARROW-5790: [Python] Raise error when trying to convert 0-dim array in pa.array Jul 10, 2019
@kszucs
Copy link
Member

kszucs commented Jul 10, 2019

CI error is unrelated.

@kszucs kszucs closed this in 167bad1 Jul 10, 2019
@jorisvandenbossche jorisvandenbossche deleted the ARROW-5790 branch July 10, 2019 12:22
wesm pushed a commit that referenced this pull request Jul 13, 2019
…n pa.array

https://issues.apache.org/jira/browse/ARROW-5790

In the `NumPyConverter` constructor, we access the strides of the array with `PyArray_STRIDES(arr_)[0]`, so need to ensure that the array is 1D before passing it there.

Author: Joris Van den Bossche <jorisvandenbossche@gmail.com>

Closes #4837 from jorisvandenbossche/ARROW-5790 and squashes the following commits:

cc66365 <Joris Van den Bossche> ARROW-5790:  raise error when trying to convert 0-dim array in pa.array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants