BUG: read_parquet, to_parquet for s3 destinations
State: closed

- [x] closes #19134
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`

closes #19134 

Labels:
name: IO Data
description: IO issues that don't fit into a more specific label
name: IO Parquet
description: parquet, feather

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19135?src=pr&el=h1) Report
> Merging [#19135](https://codecov.io/gh/pandas-dev/pandas/pull/19135?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/8acdf801c501a0ce2ac14ddd676cd248f0f32180?src=pr&el=desc) will **increase** coverage by `0.04%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #19135      +/-   ##
==========================================
+ Coverage   91.52%   91.56%   +0.04%     
==========================================
  Files         147      148       +1     
  Lines       48775    48882     +107     
==========================================
+ Hits        44639    44759     +120     
+ Misses       4136     4123      -13
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.94% <100%> (+0.04%)` | :arrow_up: |
| #single | `41.68% <33.33%> (+0.06%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19135?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2NvbW1vbi5weQ==) | `69.06% <100%> (ø)` | :arrow_up: |
| [pandas/io/s3.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3MzLnB5) | `86.36% <100%> (+1.36%)` | :arrow_up: |
| [pandas/io/parquet.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnF1ZXQucHk=) | `71.55% <100%> (+1.65%)` | :arrow_up: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `87.98% <0%> (-0.6%)` | :arrow_down: |
| [pandas/core/strings.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==) | `98.17% <0%> (-0.3%)` | :arrow_down: |
| [pandas/core/reshape/melt.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZWx0LnB5) | `97.19% <0%> (-0.06%)` | :arrow_down: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `90.6% <0%> (-0.04%)` | :arrow_down: |
| [pandas/errors/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2Vycm9ycy9fX2luaXRfXy5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [pandas/core/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY2F0ZWdvcmljYWwucHk=) | `95.78% <0%> (ø)` | :arrow_up: |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `96.77% <0%> (ø)` | :arrow_up: |
| ... and [19 more](https://codecov.io/gh/pandas-dev/pandas/pull/19135/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19135?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/19135?src=pr&el=footer). Last update [8acdf80...9dbc77c](https://codecov.io/gh/pandas-dev/pandas/pull/19135?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hello @maximveksler! Thanks for updating the PR.











Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 17, 2018 at 10:47 Hours UTC
@TomAugspurger any help on why unit test can't find pyarrow / fastparquet ?
Moto had some issues yesterday. Looking into it in a bit.

Looks like there are some listing errors too.

________________________________
From: Maxim Veksler <notifications@github.com>
Sent: Tuesday, January 9, 2018 1:21:17 AM
To: pandas-dev/pandas
Cc: Tom Augspurger; Mention
Subject: Re: [pandas-dev/pandas] Fixes S3 to_parquet write to new path (#19135)


@TomAugspurger<https://github.com/tomaugspurger> any help on why unit test can't find pyarrow / fastparquet ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/pandas-dev/pandas/pull/19135#issuecomment-356200624>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIv72IMcFH-b9uk7XSJiGi4c_qIbuks5tIxNtgaJpZM4RWU04>.

Doesn't look like a moto issue, more like a unit test environment configuration. But I might be wrong here.. had just a quick glance and couldn't spot the issue,
More specifically 

```
================================== FAILURES ===================================
______________________ TestIntegration.test_s3_roundtrip ______________________
[gw0] win32 -- Python 3.6.3 C:\Miniconda3_64\envs\pandas\python.exe
self = <pandas.tests.io.test_s3.TestIntegration object at 0x000000E2853F0400>
    def test_s3_roundtrip(self):
        expected = pd.DataFrame({'A': [1, 2, 3], 'B': 'foo'})
    
        boto3 = pytest.importorskip('boto3')
        moto = pytest.importorskip('moto')
    
        with moto.mock_s3():
            conn = boto3.resource("s3", region_name="us-east-1")
            conn.create_bucket(Bucket="pandas-test")
    
>           expected.to_parquet('s3://pandas-test/test.parquet')
pandas\tests\io\test_s3.py:24: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas\core\frame.py:1680: in to_parquet
    compression=compression, **kwargs)
pandas\io\parquet.py:227: in to_parquet
    return impl.write(df, path, compression=compression, **kwargs)
pandas\io\parquet.py:110: in write
    path, _, _ = get_filepath_or_buffer(path, mode='wb')
pandas\io\common.py:199: in get_filepath_or_buffer
    from pandas.io import s3
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
    """ s3 support for remote file interactivity """
    from pandas import compat
    try:
        import s3fs
        from botocore.exceptions import NoCredentialsError
    except:
>       raise ImportError("The s3fs library is required to handle s3 files")
E       ImportError: The s3fs library is required to handle s3 files
pandas\io\s3.py:7: ImportError
```
yeah this parameter should not be in he common call rather in the s3 specific 
Looks like you'll need to skip the test if the engine isn't available. I *think* that `pyarrow` understands `S3FileSystem` objects, so you should be able to parametrize on `engine=['fastparquet', 'pyarrow']`.
You should just have to accept an `engine` argument, and it'll pick up that fixture.
Hey guys,

It's my 3rd attempt to fix the unit test. Not too excited to spam commit history with CI a/b testing attempts :) 

Any way to reproduce the failing CI locally?
@TomAugspurger cool, looking into and thanks! 

Q - If tests are failing because code can't find pyarrow / fastparquet and the fixture will cause the test to **skip** the unit test when it can't be found then... won't it defeat the whole purpose of the unit test ?!

Can't we just we have the CI server install pyarrow/fastparquet and make it available to the pytest instead?
The failure at https://circleci.com/gh/pandas-dev/pandas/8985#tests/containers/2 was because pyarrow / fastparquet weren't installed, and the test wasn't skipped. If you take an engine argument (and pass it through to the writer / reader) it should be OK.

To test locally, uninstall both pyarrow and fastparquet. The test should be skipped.
> Can't we just we have the CI server install pyarrow/fastparquet and make it available to the pytest instead?

We do on some of our builds. But we also need to make sure pandas works without pyarrrow / fastparquet, so not all of our builds have them installed.
Tom, regarding your comment on pyarrow understanding S3FileSystem. 

I think you're right, because fastparquet seems to not understand them.

I'm getting 

```
============================= test session starts =============================
platform win32 -- Python 3.6.3, pytest-3.3.2, py-1.5.2, pluggy-0.6.0
rootdir: C:\Users\maxim.veksler\source\pandas, inifile: setup.cfg
collected 2 items
test_parquet.py F
pandas\tests\io\test_parquet.py:490 (TestIntegrationWithS3.test_s3_roundtrip[fastparquet])
self = <pandas.tests.io.test_parquet.TestIntegrationWithS3 object at 0x00000211F2652320>
df_compat =    A    B
0  1  foo
1  2  foo
2  3  foo
s3_resource = s3.ServiceResource(), engine = 'fastparquet'

    def test_s3_roundtrip(self, df_compat, s3_resource, engine):
        # GH #19134
>       df_compat.to_parquet('s3://pandas-test/test.parquet', engine)

test_parquet.py:493: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\core\frame.py:1680: in to_parquet
    compression=compression, **kwargs)
..\..\io\parquet.py:227: in to_parquet
    return impl.write(df, path, compression=compression, **kwargs)
..\..\io\parquet.py:200: in write
    compression=compression, **kwargs)
..\..\..\venv\lib\site-packages\fastparquet\writer.py:802: in write
    compression, open_with, has_nulls, append)
..\..\..\venv\lib\site-packages\fastparquet\writer.py:687: in write_simple
    with open_with(fn, mode) as f:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

f = <S3File pandas-test/test.parquet>, mode = 'wb'

    def default_open(f, mode='rb'):
>       return open(f, mode)
E       TypeError: expected str, bytes or os.PathLike object, not S3File

..\..\..\venv\lib\site-packages\fastparquet\util.py:44: TypeError
.                                                       [100%]

================================== FAILURES ===================================
____________ TestIntegrationWithS3.test_s3_roundtrip[fastparquet] _____________

self = <pandas.tests.io.test_parquet.TestIntegrationWithS3 object at 0x00000211F2652320>
df_compat =    A    B
0  1  foo
1  2  foo
2  3  foo
s3_resource = s3.ServiceResource(), engine = 'fastparquet'

    def test_s3_roundtrip(self, df_compat, s3_resource, engine):
        # GH #19134
>       df_compat.to_parquet('s3://pandas-test/test.parquet', engine)

test_parquet.py:493: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\core\frame.py:1680: in to_parquet
    compression=compression, **kwargs)
..\..\io\parquet.py:227: in to_parquet
    return impl.write(df, path, compression=compression, **kwargs)
..\..\io\parquet.py:200: in write
    compression=compression, **kwargs)
..\..\..\venv\lib\site-packages\fastparquet\writer.py:802: in write
    compression, open_with, has_nulls, append)
..\..\..\venv\lib\site-packages\fastparquet\writer.py:687: in write_simple
    with open_with(fn, mode) as f:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

f = <S3File pandas-test/test.parquet>, mode = 'wb'

    def default_open(f, mode='rb'):
>       return open(f, mode)
E       TypeError: expected str, bytes or os.PathLike object, not S3File

..\..\..\venv\lib\site-packages\fastparquet\util.py:44: TypeError
------------------------------ Captured log call ------------------------------
core.py                    203 DEBUG    Open S3 connection.  Anonymous: False
===================== 1 failed, 1 passed in 9.85 seconds ======================Exception ignored in: <bound method S3File.__del__ of <S3File pandas-test/test.parquet>>
Traceback (most recent call last):
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\s3fs\core.py", line 1219, in __del__
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\s3fs\core.py", line 1200, in close
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\s3fs\core.py", line 936, in _call_s3
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\s3fs\core.py", line 170, in _call_s3
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\client.py", line 317, in _api_call
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\client.py", line 602, in _make_api_call
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\endpoint.py", line 143, in make_request
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\endpoint.py", line 168, in _send_request
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\endpoint.py", line 152, in create_request
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\hooks.py", line 227, in emit
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\hooks.py", line 210, in _emit
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\signers.py", line 90, in handler
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\signers.py", line 154, in sign
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\auth.py", line 420, in add_auth
  File "C:\Users\maxim.veksler\source\pandas\venv\lib\site-packages\botocore\auth.py", line 354, in add_auth
ImportError: sys.meta_path is None, Python is likely shutting down
```

Should I just add an exception in the `FastParquetImpl` for when attempting to a write operation to s3?
Hmm, what version of s3fs and fastparquet do you have locally? I pulled your branch, added the `engine` argument, and they both pass for me.

```python
pytest /Users/taugspurger/Envs/pandas-dev/lib/python3.6/site-packages/pandas/pandas/tests/io/test_parquet.py -k test_s3_roundtrip -v -rsx
=================================================================== test session starts ====================================================================
platform darwin -- Python 3.6.1, pytest-3.3.1, py-1.5.2, pluggy-0.6.0 -- /Users/taugspurger/Envs/pandas-dev/bin/python3.6
cachedir: .cache
rootdir: /Users/taugspurger/Envs/pandas-dev/lib/python3.6/site-packages/pandas, inifile: setup.cfg
plugins: xdist-1.15.0, rerunfailures-2.2, repeat-0.4.1, cov-2.5.1, annotate-1.0.0, hypothesis-3.44.9
collected 44 items

pandas/tests/io/test_parquet.py::TestIntegrationWithS3::test_s3_roundtrip[fastparquet] PASSED                                                        [ 50%]
pandas/tests/io/test_parquet.py::TestIntegrationWithS3::test_s3_roundtrip[pyarrow] PASSED                                                            [100%]

=================================================================== 42 tests deselected ====================================================================
========================================================= 2 passed, 42 deselected in 1.70 seconds ==========================================================
```
FYI, you may want to run `flake8 pandas/tests/io/test_parquet.py`. That'll catch any linting errors. I think you need  another newline before your new test class.
```
fastparquet: 0.1.3
pyarrow: 0.8.0
```

obtained with `pd.show_versions()`

I find it odd that the test is passing for you. It should have failed because I now discovered `FastParquetImpl.write` is also missing the `get_filepath_or_buffer(path, mode='wb')` fix. 

Could you please verify that the `test_s3_roundtrip(self, df_compat, s3_resource, engine)` is invoked twice on your end? (once for `pyarrow` and once for `fastparquet`). Another difference might be that I'm on a Windows machine (don't ask... :/ )

Last - thanks for the flake8 tip, will do now.
`s3fs: 0.1.2` (sorry, it's getting late)
My apologies, I forgot to pass `engine` through to the readers / writers. Fastparquet does indeed fail (probably since it's `.write` needs the `mode` keyword to go through as well).

Here's my current diff.

```diff
+
 class TestIntegrationWithS3(Base):
-    def test_s3_roundtrip(self, df_compat, s3_resource):
+    def test_s3_roundtrip(self, df_compat, s3_resource, engine):
         # GH #19134
-        df_compat.to_parquet('s3://pandas-test/test.parquet')
+        key = 's3://pandas-test/test-{}.parquet'.format(engine)
+        df_compat.to_parquet(key, engine=engine)

         expected = df_compat
-        actual = pd.read_parquet('s3://pandas-test/test.parquet')
+        actual = pd.read_parquet(key, engine=engine)

         tm.assert_frame_equal(expected, actual)
-
```
  
Yeah, fastparquet fails even with the fix. 

```
f = <S3File pandas-test/test.parquet>, mode = 'wb'

    def default_open(f, mode='rb'):
>       return open(f, mode)
E       TypeError: expected str, bytes or os.PathLike object, not S3File

..\..\..\venv\lib\site-packages\fastparquet\util.py:44: TypeError
```

I think i'll leave it as a known limitation.
@maximveksler this is doable. In the fastparquet writer, do

```diff
-        path, _, _ = get_filepath_or_buffer(path)
+        path, _, _ = get_filepath_or_buffer(path, mode='wb')
         with catch_warnings(record=True):
             self.api.write(path, df,
-                           compression=compression, **kwargs)
+                           compression=compression,
+                           open_with=lambda path, mode: path,
+                           **kwargs)
```

That's basically telling fastparquet "we already have an open file, just use it".
  
@TomAugspurger please see latest commit, I've added fastparquet write and it seems to work (had to disable snappy) but read I can't seem to be able to get to work without changes in `api.py` within fastparquet itself.
@martindurant what's the easiest way to open a `fastparquet.ParquetFile(fn)` where file is an `S3File`? Or does it have to be a str? `fastparquet.ParquetFile(file, open_with=lambda path, mode: path)` doesn't quite work.
No, there is no way to simply pass a file-file object to `fastparquet.ParquetFile`, since in general we are looking for multiple files and/or metadata.
```
fastparquet.ParquetFile(fn, open_with=s3.open)
```
where s3 is a S3FileSystem.
You could have any function of the form `func(fn, mode)`.
  
rebase on master pls
@jreback @TomAugspurger any more feedback? 

_LGTM (let's get this merged)_
lgtm. ping when changed & green.
@maximveksler keep in mind, PR's can take a while to get merged. we have quite a lot and quite a bit of activity. All PR's need review and feedback time.
@jreback got ya, NP..
lgtm @maximveksler ping on green (may be a while as travis is currently in backlog on mac biulds)