Deprecate read_feather nthreads argument + update feather-format to pyarrow.feather
State: closed

- [x] closes #23053, closes https://github.com/pandas-dev/pandas/issues/21639
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: IO Data
description: IO issues that don't fit into a more specific label
name: Deprecate
description: Functionality to remove in pandas

Comments:
Hello @ingwinlu! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/io/feather_format.py`](https://github.com/ingwinlu/pandas/blob/de20f6412d9d0b7b3b1214a9ec9ff9d4f141d25c/pandas/io/feather_format.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/test_feather.py`](https://github.com/ingwinlu/pandas/blob/de20f6412d9d0b7b3b1214a9ec9ff9d4f141d25c/pandas/tests/io/test_feather.py) !



##### Comment last updated on October 15, 2018 at 05:42 Hours UTC
this would need a whatsnew note
and i believe this is tested so pls update the tests and catch the warning 
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=h1) Report
> Merging [#23112](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/9019582c45d17035098941a666a8b6072c13a357?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `83.33%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/23112/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #23112      +/-   ##
==========================================
+ Coverage   92.21%   92.22%   +<.01%     
==========================================
  Files         161      161              
  Lines       51187    51191       +4     
==========================================
+ Hits        47202    47210       +8     
+ Misses       3985     3981       -4
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <16.66%> (-0.05%)` | :arrow_down: |
| #single | `42.26% <83.33%> (+0.04%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/feather\_format.py](https://codecov.io/gh/pandas-dev/pandas/pull/23112/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2ZlYXRoZXJfZm9ybWF0LnB5) | `89.74% <83.33%> (+12.6%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=footer). Last update [9019582...e7d576d](https://codecov.io/gh/pandas-dev/pandas/pull/23112?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I test that the warning is issued now. Also rebased and followed the commit message guidelines. Added an whatsnew entry as well.
Shouldn't it be replaced with `use_threads` ?
this would require pinning pyarrow > 0.10.0 as a dependency
@jreback can you have another look?
> does this emit any warnings in the tests?

None, but ci does not test with feather 0.3.1 AFAIK.

@jreback can we directly depend on pyarrow without getting there via feather? That would make it simpler.

I am not sure if going over feather + pyarrow combinations and checking them all for how to call them is the way to go.

feather-format, 0.3.1 - no pyarrow - does not support any args
feather-format, 0.4.0 - pyarrow > 0.4.0 - does support nthreads
feather-format, 0.4.0 - pyarrow >= 0.11.0 - does not support nthreads, supports use_threads.

So the current implementation with import pyarrow should not be merged as it would not play nice in case someone is still on feather-format 0.3.1.

//edit: maybe ask feather to release a new version that pins to a higher pyarrow version...
@ingwinlu actually I will revise the though above. Am ok with dropping support for feather entirely and just using pyarrow here. Can you revise?
Sure. Will probably work on it later today.
@jreback do you want to depend on pyarrow 0.4.0 or go for 0.11.0 directly?
if u can use our min version of pyarrow (we use for parquet)

we could bump that slightly also but.l not past 0.8.0
@jreback done. don't think the windows test fail is related to the changes in the PR.
This is still missing the removal of feather-format refs in ci configs as well as some doc entry in source/io I missed.
If dropping feather, will also close #21639
I did not rewrite the io.feather section of the docs. I feel like the currently linked repository (feather) provides more information.

If you feel like it is necessary we can add a point to the caveats listed where we express that we directly require the upstream pyarrow library.
going to merge after passing: https://github.com/pandas-dev/pandas/pull/23281, which you will need to revert the xfails.
can you rebase this and check changes in #23281 
Rebased and reactivated part of the disabled tests from #23281. Did not check if the rest could also be resolved by the changes in this PR (parquet + gbq).
can you rebase and fixup?
I can not replicate the 
```
>   import pyarrow.compat as compat
E   AttributeError: module 'pyarrow' has no attribute 'compat'
```
issues locally. Any pointers?
Maybe a different version of pyarrow? https://travis-ci.org/pandas-dev/pandas/jobs/446626253#L2219
I replicated the conda environment of that test run. parquet tests run correctly but feather tests are not run at all due to an import arrow on pyarrow:
```
Python 3.6.6 |Anaconda, Inc.| (default, Oct  9 2018, 12:34:16) 
[GCC 7.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyarrow
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/winlu/miniconda3/envs/pandas/lib/python3.6/site-packages/pyarrow/__init__.py", line 60, in <module>
    from pyarrow.lib import cpu_count, set_cpu_count
ImportError: /home/winlu/miniconda3/envs/pandas/lib/python3.6/site-packages/pyarrow/../../../libparquet.so.1: undefined symbol: _ZN5boost13match_resultsIN9__gnu_cxx17__normal_iteratorIPKcSsEESaINS_9sub_matchIS5_EEEE12maybe_assignERKS9_
>>> 
```

~~Could that be some caching issue?~~

Was not using boost from conda-forge and hence had a missmatch in ABI's.

The other issue with compat seems to be related to https://github.com/apache/arrow/pull/2634/ which I will test now.
@jreback should be good now.
nthreads=0 makes some fun results >.>
@ingwinlu thanks for the responsiveness. @jorisvandenbossche over to you.
can you rebase once more.
done
@ingwinlu Thanks a lot!