ENH: Add IntegerArray.__arrow_array__ for custom conversion to Arrow
State: closed

Adding custom conversion of IntegerArray to an Arrow array, which makes that this can also be written to parquet. 
Currently it is only one way, for read_parquet it will come back as int or float (depending on presence of missing values), but fixing this is also being discussed (https://issues.apache.org/jira/browse/ARROW-2428).

The tests currently require the master version of Arrow, which we don't test. I can assure that the tests pass locally for me, but is this something we want to merge without coverage on CI? 
(in principle we could add Arrow master in eg the numpy-dev build or another one, but that is a bit more work, so not sure that is worth the currently limited number of tests we have relying on arrow)


Labels:
name: Enhancement
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
No preference on CI. If there are nightly binaries available then OK with adding them to the numpydev build.

Question: does `pa.array(obj)` call `obj.__arrow_array__` automatically?
> Question: does pa.array(obj) call obj.__arrow_array__ automatically?

What do you mean with "automatically" ? It will call it if the method exists, and for Series it will also check if the underlying values has the method (you can see the exact implementation here: https://github.com/apache/arrow/pull/5106)
Yeah, that's what I meant by automatically (rather than going through `__iter__`).
Do we just want to take on array as a dependency? I know that discussion has come up in the past without resolution. I don't object to it so if so could simplify some things here
For this PR `pyarrow` does not need to be a hard dependency, and actually here a lazy import is enough. It's only for https://github.com/pandas-dev/pandas/pull/28371 that a lazy import is more problematic (but that still doesn't need to make it a hard dependency).

Since those PRs are about IO to pyarrow, you only need this if you are actually using pyarrow for others reasons (eg for IPC, for parquet, ..), not for internal functionality in pandas. So I would keep the discussion of making pyarrow a hard dependency for later if we would make more use of it in pandas itself.
#28371 does something similar for Period and Interval.  Are there more coming after this?  Would it make sense to collect these somewhere like compat._arrow?  I don't have a problem with the placement in this PR, just thinking out loud.
> Would it make sense to collect these somewhere like compat._arrow? 

The actual `__arrow_array__` of course needs to be defined on the actual IntegerArray, but the implementation itself could call out to another module. For the type classes I am defining in #28371 this might make sense, but for this PR the implementation is literally one line (`pa.array(self._data, mask=self._mask, type=type)`), so IMO that is not worth putting in a separate function somewhere else.

But let's discuss further in #28371
Any code comments? 
I think this one is good to go, #28371 needs more discussion though (where to put it, always try to import pyarrow or not, ..)
>  So is everyone else aligned on this dunder for conversion?

Well, it's a protocol of pyarrow, not pandas, and the decision is made in pyarrow now (it would be like pandas not liking numpy's `__array__`). Of course, feedback on the protocol for conversion to arrow is certainly welcome, it's very new so can still be changed.
>  it's a protocol of pyarrow, 

For context, see also https://github.com/pandas-dev/pandas/issues/20612#issuecomment-530749254
Not at a computer to check but I think it returns the module or None, so you can use that instead of rolling your own ZZZ_INSTALLED globals 

Sent from my iPhone

> On Sep 12, 2019, at 10:59 AM, Joris Van den Bossche <notifications@github.com> wrote:
> 
> @jorisvandenbossche commented on this pull request.
> 
> In pandas/tests/arrays/test_integer.py:
> 
> > @@ -19,6 +21,13 @@
>  from pandas.tests.extension.base import BaseOpsUtil
>  import pandas.util.testing as tm
>  
> +try:
> I've never used compat._optional.import_optional_dependency before, so correct if me if I am wrong: it seems that method is typically used in the code (not tests) and is meant to raise an error or return the module (so eg in functions that use an optional dependency).
> So I would still need to catch the error, I think? In which case I am not sure it is necessarily clearer, or deduplicating code.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

Probably want a release note? Or... perhaps not since pyarrow 0.15 isn't out yet?

Feel free to merge if a release note isn't needed.
Thanks.