ENH: add fsspec support
State: closed

Supersedes https://github.com/pandas-dev/pandas/pull/33549
closes #33452

- [x] closes #xxxx
- [x] tests added / passed
- [x] passes `black pandas`
- [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

Comments:
I see some parquet changes here, will need a bit of work...
Hello @martindurant! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





















There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-06-19 18:15:56 UTC
(NB: some failures are due to v0.7.4 of fsspec only being on conda-forge)
but I will need some help with
```
pandas/tests/io/test_fsspec.py:18: error: Item "None" of "Optional[str]" has no attribute "encode"
```
(i.e., I know that, given the input, the output is str and not None, but how do I tell mypy that)
> Linting error: https://github.com/pandas-dev/pandas/pull/34266/checks?check_run_id=697447968#step:11:10

I need help with this. The return of `to_csv` is Optional[str], but for the input I gave it, it must be str, so that encode is a legal thing to do. How do I specify that? Wrapping it in `str()` does not seem right. 

> CI failures

There runs are using an older fsspec from conda defaults. I can ask the conda team to update the version, but we should be certain that this PR doesn't require any further changes in fsspec.
Note: fsspec 0.7.4 will be up on conda defaults soon. At that point I'll remove the WIP tag.
Those two points were fixed on my local branch, not yet pushed.
Remaining issues: 
- failure on parsing datetime, yes in S3 test (but I don't get how my changes could have caused this)
- the mypy failure on `df.to_csv().encode()`
- some envs still having old gcsfs, but tests only skipping if there is no gcsfs at all. Should both update skipping and envs.
Oh, and one more: the use of `"s3n://"` as a protocol. This is not listed as a protocol for S3 in s3fs, but "s3a" is. Is it really needed? A [good description](https://stackoverflow.com/a/33356421/3821154) of what the protocol specifiers mean to hadoop.
s3n comes from https://github.com/pandas-dev/pandas/pull/11071. I'm not sure if it's in use, but I'd like to avoid changing it without warning.
fsspec does not yet have an entrypoints method for declaring backends, so I'll have to both add s3n as a "known protocol" and also add it to the list of possible protocols in S3FileSystem, and release both.
OK, that only leaves the localised timestamps. Any suggestions?
Are you able to reproduce the failures locally? To get the exact versions used in the env, you can `conda env create --file=ci/deps/azure-36-locale.yaml`.

The locale setting can be a bit trickier. That test is job is I think it here: https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=36700&view=logs&j=98c84d08-34d8-513e-80be-2c581992dd5a&t=db7ca50c-72c8-5416-b299-59f9d25bdff8&l=19 to `it_IT.utf8`. On my Mac I can achieve the same effect (I think) by setting LANG

```
$ LANG=it_IT.UTF-8 python -c 'import locale; print(locale.getlocale())'
('it_IT', 'UTF-8')
```
Additional note: https://github.com/pandas-dev/pandas/pull/34500 should go in first I think (otherwise that will be hard to backport)
@TomAugspurger , the following passes for me locally
```
LANG=it_IT.UTF-8 pytest pandas/tests/io/test_fsspec.py::test_from_s3_csv
```
@martindurant is that in an environment created from the environment file in `ci/deps`? And are you able to step through and see where things go wrong? If not I can take a look in a few days.

I think our setup also does some hacky stuff to set the locale: https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=36798&view=logs&j=a3a13ea8-7cf0-5bdb-71bb-6ac8830ae35c&t=2296ffae-5e67-52bf-66a8-6160145a0f80&l=14, but I hoped that wouldn't be needed.

There might be some additional failures. https://travis-ci.org/github/pandas-dev/pandas/jobs/695142967#L1480 indicates that a file handle may not be being closed correctly.

```
 ResourceWarning: unclosed file <_io.TextIOWrapper name=0 mode='r' encoding='UTF-8'>

sys:1: ResourceWarning: unclosed <socket.socket fd=8, family=AddressFamily.AF_INET, type=2050, proto=0, laddr=('0.0.0.0', 0)>

=================================== FAILURES ===================================

________________________ TestPandasContainer.test_to_s3 ________________________

[gw0] linux -- Python 3.6.6 /home/travis/miniconda3/envs/pandas-dev/bin/python

self = <pandas.tests.io.json.test_pandas.TestPandasContainer object at 0x7f796ca6e908>

s3_resource = s3.ServiceResource()

    def test_to_s3(self, s3_resource):

        # GH 28375

        mock_bucket_name, target_file = "pandas-test", "test.json"

        df = DataFrame({"x": [1, 2, 3], "y": [2, 4, 6]})

        df.to_json(f"s3://{mock_bucket_name}/{target_file}")

>       assert target_file in (

            obj.key for obj in s3_resource.Bucket("pandas-test").objects.all()

        )

E       AssertionError: assert 'test.json' in <generator object TestPandasContainer.test_to_s3.<locals>.<genexpr> at 0x7f796c5c6518>

pandas/tests/io/json/test_pandas.py:1672: AssertionError
```
FYI, @martindurant I am only able to reproduce setting in the init

```diff
diff --git a/pandas/__init__.py b/pandas/__init__.py
index d6584bf4f1..1a4147f49d 100644
--- a/pandas/__init__.py
+++ b/pandas/__init__.py
@@ -1,4 +1,6 @@
 # flake8: noqa
+import locale
+locale.setlocale(locale.LC_ALL, 'zh_CN.UTF-8')
 
 __docformat__ = "restructuredtext"
```

In particular, I'm not able to reproduce by setting the LC_ALL, etc. variables

```
$ LC_ALL=zh_CN.UTF-8 LANG=zh_CN.UTF-8 pytest pandas/tests/io/test_fsspec.py::test_from_s3_csv -x --pdb
```

And it's boto that's failing to parse the timestamp. I'm inclined to think that this is an artifact of our test setup. We shouldn't be setting that in the init, since it isn't indicative of real-world usage, but I'd like to better understand why that value is causing issues for boto first.
Well, after [some reading](https://docs.python.org/3/library/locale.html#background-details-hints-tips-and-caveats), I'm inclined to say that our tests are "wrong". Python apparently always uses the C locale, even when `LANG`, etc. are set.

I can also reproduce the error on master with a simple test like

```python
import pandas as pd
import locale
locale.setlocale(locale.LC_ALL, 'zh_CN.UTF-8')


def test_foo(s3_resource):
    assert s3_resource
    pd.read_csv('s3://pandas-test/tips.csv')
```

So for now @martindurant I'd recommend removing fsspec / s3fs from CI jobs that set the locale like that. We can investigate followups, though we'll want to make sure we aren't losing coverage on tests.

Still haven't looked at the resource warning failure.
@TomAugspurger , @jorisvandenbossche - that makes it green.
pandas failed to build? Should it just be run again?
> pandas failed to build? Should it just be run again?

i've restarted the 2 github workflow jobs. The azure restart is ignored (for me) while the other jobs are still running. will restart that later. 
Blocked by https://github.com/pandas-dev/pandas/issues/34704
Can you re-run `scripts/generate_pip_deps_from_conda.py` and commit the output?

Maybe merge master to. travis was failing.
@TomAugspurger done
As mentioned before, I would prefer to merge https://github.com/pandas-dev/pandas/pull/34500 first (to ensure this case is actually fixed/tested, and also working with those changes)
> As mentioned before, I would prefer to merge #34500 first (to ensure this case is actually fixed/tested, and also working with those changes)

sure let's do that
@martindurant this will need a merge of master now
@jorisvandenbossche , please check carefully; I am guessing at what the new API should be like. Are you certain you want to depend on latest pyarrow here?
How are we relying on the latest pyarrow? (I might have missed something in the other PR then)
Isn't that why the call changed to read_table? After pulling from master, pandas no longer builds for me, which is painful, I supposed some dep changed.
In any case, this will now need massage.