New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-9879: [Python] Add support for numpy scalars to ChunkedArray.__getitem__ #8072
Conversation
python/pyarrow/table.pxi
Outdated
elif isinstance(key, int): | ||
return self.getitem(key) | ||
elif np.isscalar(key) and np.issubdtype(key.dtype, np.integer): | ||
return self.getitem(key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative is to do like array.pxi
, and let python/cython handle the cast to integer (and let it raise error otherwise:
elif isinstance(key, int): | |
return self.getitem(key) | |
elif np.isscalar(key) and np.issubdtype(key.dtype, np.integer): | |
return self.getitem(key) | |
return self.getitem(_normalize_index(key, self.chunked_array.length())) |
(minor downside is that the error message will only speak about "integer", and not "slice", but then anything convertible to int will be accepted automatically)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should probably use the same solution everywhere (and possibly avoid manual handling of Numpy scalars).
@xhochy do you have time to update this? |
Maybe Friday or Monday |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging on green.
…getitem__ FYI @marc9595 Closes apache#8072 from xhochy/ARROW-9879 Lead-authored-by: Uwe L. Korn <uwelk@xhochy.com> Co-authored-by: Krisztián Szűcs <szucs.krisztian@gmail.com> Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
FYI @marc9595