COMPAT/REF: Use s3fs for s3 IO
State: closed

- [x] closes #11915
- [x] tests added / passed
- [x] passes `git diff upstream/master | flake8 --diff`
- [x] whatsnew entry


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

Comments:
need to add `s3fs` to `.pip` (where we have `boto` now)

This is just refactoring the reading for now. We should be able to support writing to s3 buckets pretty easily now too. A few things
- pandas accepts `s3://`, `s3n://`, and `s3a://` protocols, but s3fs only expects the regular s3://. I work around that [here](https://github.com/pydata/pandas/pull/13137/files#diff-0d7b5a2c72b4dfc11d80afe159d45ff8R251). [This](https://wiki.apache.org/hadoop/AmazonS3) was the best source I found on the differences, but I don't know if simply chaning the protocol will break anything. At the end of the day we're just pulling down bytes, which should presumably represent a CSV
- API Changes: Before we raised OSError when the s3 file wasn't found, and reraised botos PermissionError when you didn't have permission to read the file. I think s3fs exposes both of these as IOErrors [here](https://github.com/dask/s3fs/blob/master/s3fs/core.py#L631). We might be able to work around this

cc @jreback @martindurant

s3fs now includes original boto3 exceptions when something goes wrong https://github.com/dask/s3fs/pull/42

For your question above, I believe s3: and s3n: work exactly the same from our point of view; but we don't touch the block filesystem emulation (this is ancient, I don't know if it's used in the wild).

The failure above appears to be a typo:

```
File "/home/travis/build/pydata/pandas/pandas/io/parsers.py", line 36, in <module>
    need_text_wrapping = (compat.BtyesIO,)
AttributeError: module 'pandas.compat' has no attribute 'BtyesIO'
```

I haven't yet tried this PR, but I've had some issues with s3fs lately [1], and I'd like to try this out before it gets merged. Assuming liberal permissions on s3 buckets and objects and dealing with profiles has been a bit of an issue across third-party tools for me lately. It could be an issue with permissions, but I suspect it's going to continue to come up.

E.g., I can do this

```
import boto3
session = boto3.Session(profile_name='myprofile')
s3 = session.client('s3')
key = s3.get_object(Bucket=BUCKET, Key=KEY)
```

but not 

```
import s3fs
fs = s3fs.S3FileSystem(profile_name='myprofile')
f = fs.open('s3://' + BUCKET + '/' + KEY)
```

and I don't see a way to set the profile explicitly in S3File and it doesn't seem to matter if I set my `AWS_PROFILE` correctly, but I haven't gone deep here.

[1] https://github.com/dask/s3fs/issues/38

@jseabold yeah, I've been hitting the same issue. For your specific case, maybe try

``` python
import s3fs
fs = s3fs.S3FileSystem(profile_name='myprofile', anon=False)
f = fs.open('s3://' + BUCKET + '/' + KEY)
```

If I use `anon=False` though in pandas though, I'm unable to read from publicly available files like `s3://pandas-test/tips.csv'. With`anon=None` (the default) I can't read from private buckets.

I spent last night trying to figure out a way to handle both, no luck yet.

Ah, sure. Isn't there a try/except for that in the boto code that's removed by this PR?

I'm just really worried that this is going to break everything for me. After having to downgrade dask, use a patched conda, use a patched odo, and run boto master on Python 3 until the last release, I'm super wary about these things.

this won't be till 0.19 I think we have it marked in any event

> Isn't there a try/except for that in the boto code that's removed by this PR?

Yep, I was hoping to avoid that in pandas, but it might be necessary.

@jseabold I have a bunch of stuff on AWS too and I share your concern about this breaking things.
I'll be writing many more tests. This is still a ways from being done, but I'll keep you updated.

I'm not sure if we're dealing with one issue here or more: 
- the `profile_name` keyword to S3FileSystem is designed to be passed along to boto3.Session, just like above
- connections are attempted with `anon=False` first, and then fallback to `True` if not specified;
- publicly available buckets should always be available, regardless of anon.

I don't have boto profiles, just a [default] section in my .aws/credentials (no relevant env variables). How should I go about recreating your situation? In any case, I suggest the discussion should happen over on s3fs.

@martindurant I've started today trying to collect the info for my typical permissions, so you can replicate this stuff.

Is the original motivating bug in boto fixed with the newest release? I'm not sure if it's the same config parsing one I alluded to with conda that I fixed with a monkey patch.

What's the motivation for this over using boto3? I need to read the linked thread more carefully. I'd tentatively suggest not to add reliance on another third party package and introduce some more complexity here. The `read_csv` code is already pretty hard to follow with the conflation of source location with file format and all the options without having to dig through another package in addition to the Amazon ones.

> Is the original motivating bug in boto fixed with the newest release? I'm not sure if it's the same config parsing one I alluded to with conda that I fixed with a monkey patch.

Yep, not wanting to manage the boto to boto3 switch within pandas was what initially prompted this.  
It's a tradeoff between the complexity of (re)implementing it within pandas vs. the complexity of accepting a dependency. I'll try to get something more polished done tonight.

> It's a tradeoff between the complexity of (re)implementing it within pandas vs. the complexity of accepting a dependency.

That's true, but you have control over what is getting implemented, so you don't have extra calls around things that a filesystem mimic might want, but a single stream a key to memory thing might not. To be fair, it looks like the `S3File` objects avoid some of the problems that I've been running in to with permissions.

This is probably out of scope for this PR, but I've often wanted to be able to pass a boto(3) `Key` object to `read_csv` rather than having to check how pandas is doing the session creation. If `S3File` is sufficiently file-like I'd love to skip the discovery and make me an s3 client steps, so I don't have to remind myself.

@jseabold this is to REDUCE complexity in the code. `boto3` is somewhat raw. This makes us just treat it directly as a filesystem and is pretty generic. This is how all file-ilke things should be done, abstract them away from pandas directly. (e.g.this is done implicity with compression / unicode conversions, though these should be abstracted a bit more, but they are usually just a line or 2). 

This makes it so pandas can simply use these w/o having to worry about messy details like authentication and such which the service can simply care about (and upgrade / fix as needed).

You might be interested that https://github.com/dask/hdfs3 provides the very same interface to HDFS - perhaps a less used data source for pandas, but an option that might be useful in the future. We may also make the same interface for other cloud storage services, depending on demand/complexity.

I agree with the sentiment and am all for reducing the complexity (in pandas) around csv reading, file-like objects, location discovery, and authentication around those locations.

@TomAugspurger [This](https://github.com/dask/s3fs/pull/46#issuecomment-220775582) may be what's going on with  `anon`  for you.

update for 0.19.0?

I'll revisit this mid next week. 

> On Jul 14, 2016, at 19:10, Jeff Reback notifications@github.com wrote:
> 
> update for 0.19.0?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

@TomAugspurger ?

@TomAugspurger What's the status of this?

Pushing I think. Haven't had time to work on it, and there's one more change I want to submit to s3fs.

@TomAugspurger would be great for 0.20.0!
Pushed an update, will add a bit more for the docs later. Ended up being pretty simple.
Anecdotal performance boost:

```
# first one is master
bash-3.2$ nosetests pandas/io/tests/parser/test_network.py:TestS3
..........
----------------------------------------------------------------------
Ran 10 tests in 62.720s

OK
# now on this branch
bash-3.2$ git co -
Switched to branch 's3fs'
Your branch is up-to-date with 'origin/s3fs'.
bash-3.2$ nosetests pandas/io/tests/parser/test_network.py:TestS3
..........
----------------------------------------------------------------------
Ran 10 tests in 15.995s

OK
```

4x speedup seems a bit too good to be true :)
let's do this after #14576 as that's changes the paths for the io code
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/13137?src=pr) is 84.64% (diff: 21.05%)
> Merging [#13137](https://codecov.io/gh/pandas-dev/pandas/pull/13137?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **0.07%**

```diff
@@             master     #13137   diff @@
==========================================
  Files           144        144          
  Lines         51057      51016    -41   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43180      43184     +4   
+ Misses         7877       7832    -45   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [f1cfe5b...92ac063](https://codecov.io/gh/pandas-dev/pandas/compare/f1cfe5b4eb614f9b4e201e764e9bd25f2389b355...92ac0634e33188d888e63cf04d2a49b3db25df8d?src=pr)
I think this is ready to go, if anyone has further comments.
lgtm.

@martindurant ?
No comments from me on the pandas code.
I *do* expect potential problems in the future as boto3/botocore changes internal things, but that would have been true for any wrapping. Will need to keep an eye on that.