COMPAT: reading json with lines=True from s3, xref #17200
State: closed

Attempt to decode the bytes array with `encoding` passed to the call.

 - [ ] closes #17200 


Labels:
name: IO JSON
description: read_json, to_json, json_normalize
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
@alph486 : Thanks for the PR!  You will need to write tests for us to merge this.  I would also take a look at what @TomAugspurger suggested in terms of patching this.
Is there a particular way you guys go about handling s3 in your tests? I see some files in a bucket somewhere, but idk if there are any `jsonl` type files or how to add one. For that matter, I don't see any tests for `read_json` where the file is in s3.
Hello @alph486! Thanks for updating the PR.









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

##### Comment last updated on November 27, 2017 at 00:04 Hours UTC
@gfyoung After going back to clean this up I realized jsonl from URL based file also suffers from the same issue. I have a cleaner fix that I think works in both cases.

Can you let me know how I should proceed for getting a JSONL file uploaded to the public s3 bucket for testing this in the test suite? I don't have write access.
> Can you let me know how I should proceed for getting a JSONL file uploaded to the public s3 bucket for testing this in the test suite? I don't have write access.

@TomAugspurger @jreback : thoughts?
if you put the file on a gist somewhere @TomAugspurger or I can upload to the bucket.
@jreback @gfyoung Thanks for the assist here. I'll wrap this up soon; just after I get my current project completed that requires this change.
@jreback @TomAugspurger Here's a file that I use in my tests. If you can give me an s3 path where you drop it I'll push the tests. https://gist.github.com/alph486/c7eff9a896d80dccae5a3b3283e75a4e#file-items-jsonl 
bump?
@alph486 : Are you still waiting on the updated URL to patch the failing tests?
@gfyoung yeah, i just need a target s3 url to put in my code
ping @jreback @TomAugspurger 
I don't think I have access to the pandas s3 bucket.

On Fri, Aug 18, 2017 at 1:59 PM, gfyoung <notifications@github.com> wrote:

> ping @jreback <https://github.com/jreback> @TomAugspurger
> <https://github.com/tomaugspurger>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/17201#issuecomment-323434953>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIvlpJ-cAwFzSmyAFeY270PMEynkhks5sZd78gaJpZM4OxISG>
> .
>

@jreback ?
yeah I don't think I have this either. @TomAugspurger IIRC you can login using the Continuum dashboard.
if that works we should prob change it :>
Hmm I'm not seeing it in the environment I have access to. I'll ask IT.
Probably best to have it under an account we manage anyways.

On Mon, Aug 21, 2017 at 6:56 PM, Jeff Reback <notifications@github.com>
wrote:

> if that works we should prob change it :>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/17201#issuecomment-323881918>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIjccf2YRr9hMExOy0a3VnV3HTBXBks5sahlCgaJpZM4OxISG>
> .
>

sorry to keep nudging here, but any progress from IT @TomAugspurger ? Id love to try to a patched official version out so we can use our feature in production.
No word yet.

Really, we should be using https://github.com/spulec/moto to mock all our S3 calls. Now that s3fs (and boto3) are handling all the network stuff, we don't need to be testing it. Just our text parsing. I'll open an issue about that (and you can look at fixing it if you're interested :)

https://github.com/pandas-dev/pandas/issues/17325
I can try to implement my tests using moto... we'll see if that works

Thanks,
Kevin

On Aug 24, 2017, 6:04 AM -0500, Tom Augspurger <notifications@github.com>, wrote:
> No word yet.
> Really, we should be using https://github.com/spulec/moto to mock all our S3 calls. Now that s3fs (and boto3) are handling all the network stuff, we don't need to be testing it. Just our text parsing. I'll open an issue about that (and you can look at fixing it if you're interested :)
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

Thanks, I'd be totally fine with just using moto for your test for now, and following up moving most of the other S3 tests to use moto.

You'll need to edit one of the ci files to include moto. Probably `ci/requirements-3.6.run`
Ah, thanks for that. I hope to get to trying that locally today, I'll update here when I know.

Thanks,
Kevin

On Aug 24, 2017, 8:28 AM -0500, Tom Augspurger <notifications@github.com>, wrote:
> Thanks, I'd be totally fine with just using moto for your test for now, and following up moving most of the other S3 tests to use moto.
> You'll need to edit one of the ci files to include moto. Probably ci/requirements-3.6.run
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

Any guidance on which requirements files I should change for both release packaging, development, and CI? (I currently have `ci/requirements-3.6.run` and `requirements_dev.txt`). I just have to add `s3fs` to some things and `moto` where appropriate.
you shouldn't add ``s3fs`` anywhere
Unless I'm misunderstanding: you cannot run any of the test suites that require `pandas.io.s3` locally unless s3fs is part of `requirements_dev.txt` no?

Edit:

For example from my attempt of running `pytest pandas/tests/io/json/test_pandas.py -v`:

```

pandas/tests/io/json/test_pandas.py:996:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas/io/json/json.py:324: in read_json
    encoding=encoding)
pandas/io/common.py:208: 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
=====================================
```
I think Jeff was saying that you shouldn't need to add s3fs to any of the `ci/requirements*` files.
I think just add moto anywhere there where you see s3fs. You could add it to `ci/requirements_all.txt` as well.

The tests should be skipped if s3fs and moto are not present (`pytest.importorskip`)
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=h1) Report
> Merging [#17201](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/3c833db29b6f5977c78d1ade791a09a5b29cedb8?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `25%`.

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

```diff
@@            Coverage Diff             @@
##           master   #17201      +/-   ##
==========================================
- Coverage   91.01%   90.99%   -0.02%     
==========================================
  Files         162      162              
  Lines       49502    49504       +2     
==========================================
- Hits        45053    45045       -8     
- Misses       4449     4459      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.77% <25%> (-0.01%)` | :arrow_down: |
| #single | `40.27% <25%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2lvL2NvbW1vbi5weQ==) | `67.64% <25%> (-0.15%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.71% <0%> (-0.1%)` | :arrow_down: |

------

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

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=h1) Report
> Merging [#17201](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/674fb96b33c07c680844f674fcdf0767b6e3c2f9?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #17201      +/-   ##
==========================================
- Coverage   91.35%   91.33%   -0.02%     
==========================================
  Files         163      163              
  Lines       49801    49801              
==========================================
- Hits        45496    45487       -9     
- Misses       4305     4314       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.13% <ø> (ø)` | :arrow_up: |
| #single | `40.79% <ø> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/json/json.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vanNvbi5weQ==) | `100% <ø> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17201?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |

------

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

@TomAugspurger Once accepted how long does it typically take for a patch version to be released for public use? Wondering how soon I can consume the change assuming its accepted.
We're targeting end of September for the next release (probably a release
candidate first, then a full release).

You can check our milestones at
https://github.com/pandas-dev/pandas/milestones (we are decent about
hitting the target dates).

On Thu, Sep 21, 2017 at 9:27 AM, Kevin Kuhl <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/tomaugspurger> Once accepted how long
> does it typically take for a patch version to be released for public use?
> Wondering how soon I can consume the change assuming its accepted.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/17201#issuecomment-331173601>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIlarSyDyV9s7ROjALixKXn_miKdxks5sknJtgaJpZM4OxISG>
> .
>