Storage options #35381

martindurant · 2020-07-22T15:47:31Z

martindurant · 2020-07-23T14:25:50Z

Refs: #28377 , #30359 , #33639 , #16692 (and probably more)

martindurant · 2020-07-23T15:51:57Z

This PR is near complete.
There may be more places where storage_options can be added, but the most important cases are covered and tested.

Question: the docstring for the kwarg should be added, but it would be repeated verbatim in every place; is this the right thing to do?

(cc @TomAugspurger )

TomAugspurger · 2020-07-23T18:58:57Z

Question: the docstring for the kwarg should be added, but it would be repeated verbatim in every place; is this the right thing to do?

Yeah I think it needs to be duplicated. We do share some sharing between some readers, but not all. It'll be easier to just add the same lines to each, and add an example in one place (perhaps read_csv).

TomAugspurger · 2020-07-23T19:01:16Z

pandas/core/frame.py

-        buf: Optional[IO[str]] = None,
-        mode: Optional[str] = None,
+        buf: Optional[Union[IO[str], str]] = None,
+        mode: Optional[str] = "wt",


Why did this need to change?

mode=None doesn't actually make sense. This is for writing, and most open commands (including python builtin) would default to "rt" or "rb".

if changing this, could also remove Optional from type annotation.

pandas/core/frame.py

TomAugspurger · 2020-07-23T19:04:15Z

pandas/io/common.py

@@ -234,6 +236,8 @@ def get_filepath_or_buffer(
            ).open()

        return file_obj, encoding, compression, True
+    elif storage_options:
+        raise ValueError("storage_options passed with non-fsspec URL")


Is the "URL" here actually a user-provided buffer?

Local file or HTTP URL; but I guess an already open file instance would end up here too, worth checking.

pandas/tests/io/test_fsspec.py

martindurant · 2020-07-24T19:06:26Z

(fails against numpy dev with "ValueError: invalid array_struct" while reading stata)

martindurant · 2020-08-06T14:55:45Z

Added text to io.rst

martindurant · 2020-08-06T14:59:40Z

I disagree with "file-like objects must be iterable" in is_file_like - most uses (in pandas) require only read, and certainly for stata.

TomAugspurger

LGTM, pending https://github.com/pandas-dev/pandas/pull/35381/files#r466474268.

TomAugspurger · 2020-08-06T15:02:45Z

IIRC we've gone back and forth on exactly what is required. I don't especially care what is required, but I'd prefer to be consistent in what we accept across readers unless there's a good reason not to be. Hopefully @gfyoung can comment here.

martindurant · 2020-08-06T15:28:43Z

isort is complaining, but passes for me locally. Can someone tell me what's wrong?

Check import format using isort
##[error]ERROR: /home/runner/work/pandas/pandas/pandas/conftest.py Imports are incorrectly sorted and/or formatted.
Check import format using isort DONE
##[error]Process completed with exit code 1.

code:

@pytest.fixture()
def fsspectest():
    pytest.importorskip("fsspec")
    from fsspec.implementations.memory import MemoryFileSystem
    from fsspec import register_implementation
    from fsspec.registry import _registry as registry

TomAugspurger · 2020-08-06T15:51:23Z

Perhaps check that the version of isort matches?

Also some warnings in io.rst: https://github.com/pandas-dev/pandas/pull/35381/checks?check_run_id=954318397#step:6:2333.

martindurant · 2020-08-06T17:32:51Z

@TomAugspurger you were right, newer isort solved it; also fixed the RST link.

martindurant · 2020-08-07T19:12:33Z

ping

TomAugspurger · 2020-08-07T19:14:51Z

My only outstanding question is #35381 (comment).

martindurant · 2020-08-07T19:26:17Z

@gfyoung has not yet responded, and I would rather not require iterable files, nor change the utils code that other things might be depending on. I suppose I'm stuck for now! This particular case is only for stata, and the previous code didn't use is_file_like either.

(note that IOBase does guarantee iterability, but I'm not certain that all file-like implementations do - not that I can imagine why they wouldn't)

jreback

looks reasonable a few comments

doc/source/user_guide/io.rst

doc/source/whatsnew/v1.2.0.rst

jreback · 2020-08-07T19:31:19Z

pandas/core/generic.py

@@ -3126,6 +3156,16 @@ def to_csv(

            .. versionadded:: 1.1.0

+        storage_options : dict, optional


i wonder if there is a way to share doc-strings components for all of these i/o methods

I am not aware of a way

yeah i think we can do this with our shared docs infra, but out of scope for now

jreback · 2020-08-07T19:32:17Z

pandas/io/formats/csvs.py

@@ -54,6 +54,7 @@ def __init__(
        doublequote: bool = True,
        escapechar: Optional[str] = None,
        decimal=".",
+        storage_options: Optional[Dict[str, Any]] = None,


seeing as we are doing this in lots of places I would add it

jreback · 2020-08-07T19:33:33Z

pandas/io/stata.py

@@ -1931,7 +1937,9 @@ def read_stata(


 def _open_file_binary_write(
-    fname: FilePathOrBuffer, compression: Union[str, Mapping[str, str], None],
+    fname: FilePathOrBuffer,
+    compression: Union[str, Mapping[str, str], None],


can also add Compression to typing.py

This wasn't me, but the styling; compression as a kwarg has different types in different places (most often str, but also Optional[Union[str, Mapping[str, Any]]], which is similar), so I don't think there's a useful common type to be made.

hmm interesting, we ought to fix that. can you create an issue about this?

I'm not certain it's wrong - sometimes the args is required, but could be None, other times it's not required, but the default value can be None or str (e.g., "infer").

Mainly defining the StorageOptions type

jreback · 2020-08-07T20:16:01Z

doc/source/whatsnew/v1.2.0.rst

@@ -21,7 +21,8 @@ to pass a dictionary of parameters to the storage backend. This allows, for
 example, for passing credentials to S3 and GCS storage. The details of what
 parameters can be passed to which backends can be found in the documentation
 of the individual storage backends (detailed from the fsspec docs for
-`builtin implementations`_ and linked to `external ones`_).
+`builtin implementations`_ and linked to `external ones`_). See


are these referenced in io.rst (more important that they are there), ok if they are here as well (but not really necessary)

I phrased it a bit differently (one general link, one specific instead of two specific) - I'll make the two places more similar.

jreback · 2020-08-07T21:38:03Z

##[error]doc/source/user_guide/io.rst:1706:83:E501:line too long (86 > 79 characters)

lint issue

martindurant · 2020-08-10T14:29:48Z

lint issue

@jreback : fixed I think. Even linting the docs...

TomAugspurger

That's to avoid horizontal scroll bars in the code snippets in the HTML rendered version.

TomAugspurger · 2020-08-10T15:19:27Z

Thanks!

Martin Durant added 5 commits July 10, 2020 11:20

Pepper storage_options

3a54dde

Merge branch 'master' into storage_options

f0922c4

Add feather test

e549f8d

Merge branch 'master' into storage_options

e8540c4

Add CSV and parquet options tests; lint

0034bff

martindurant marked this pull request as draft July 22, 2020 15:47

Martin Durant added 6 commits July 22, 2020 12:09

deeper lint

19f041d

more tests

f9e1e69

blank line

7f69afe

attempt relint

cc0e4c3

unused import

e356e93

more order

c7170dd

Martin Durant added 2 commits July 23, 2020 11:23

plumb stata and test

b96778d

Add note about storage_options in whatsnew

1dc41b1

Martin Durant added 4 commits July 23, 2020 12:00

Plumb and test markdown

d882984

optional markdown

f1e455d

remove extraneous

c88b75f

more extraneous

58481a4

TomAugspurger added API Design IO Data IO issues that don't fit into a more specific label labels Jul 23, 2020

TomAugspurger added this to the 1.2 milestone Jul 23, 2020

TomAugspurger reviewed Jul 23, 2020

View reviewed changes

Martin Durant added 4 commits July 24, 2020 13:33

Add fsspec options error and docstrings

704770b

fix that

1b8637e

black

bbcef17

fix it again

a18686c

TomAugspurger approved these changes Aug 6, 2020

View reviewed changes

sort and remove colon

32cf204

Merge branch 'master' into storage_options

682e9e5

jreback requested changes Aug 7, 2020

View reviewed changes

More suggestions

f7f086c

Mainly defining the StorageOptions type

jreback reviewed Aug 7, 2020

View reviewed changes

More IO link

b5138f0

jreback approved these changes Aug 7, 2020

View reviewed changes

doc code lint fix

afdc030

TomAugspurger approved these changes Aug 10, 2020

View reviewed changes

TomAugspurger merged commit df1d440 into pandas-dev:master Aug 10, 2020

martindurant mentioned this pull request Aug 11, 2020

Moto server #35655

Merged

5 tasks

TomAugspurger mentioned this pull request Aug 14, 2020

read_csv Pass File System (AWS, GCP) as Option. #30359

Closed

TomAugspurger mentioned this pull request Aug 28, 2020

Unable to specify AWS profile when saving parquet file with s3 url #19904

Closed

This was referenced Sep 4, 2020

use s3fs authentication if provided #33639

Closed

Read/Write files on specific S3 accounts #16692

Closed

alimcmaster1 mentioned this pull request Jan 14, 2021

QST: Why is storage_options parameter missed in read_table method? #39167

Closed

2 tasks

alimcmaster1 mentioned this pull request Aug 26, 2021

ENH: Add Storage Options kwarg to read_table #43239

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storage options #35381

Storage options #35381

martindurant commented Jul 22, 2020 •

edited

Loading

martindurant commented Jul 23, 2020 •

edited

Loading

martindurant commented Jul 23, 2020

TomAugspurger commented Jul 23, 2020

TomAugspurger Jul 23, 2020

martindurant Jul 23, 2020

simonjayhawkins Jul 24, 2020

TomAugspurger Jul 23, 2020

martindurant Jul 23, 2020

martindurant commented Jul 24, 2020

martindurant commented Aug 6, 2020

martindurant commented Aug 6, 2020

TomAugspurger left a comment

TomAugspurger commented Aug 6, 2020

martindurant commented Aug 6, 2020

TomAugspurger commented Aug 6, 2020

martindurant commented Aug 6, 2020

martindurant commented Aug 7, 2020

TomAugspurger commented Aug 7, 2020

martindurant commented Aug 7, 2020

jreback left a comment

jreback Aug 7, 2020

martindurant Aug 7, 2020

jreback Aug 7, 2020

jreback Aug 7, 2020

jreback Aug 7, 2020

martindurant Aug 7, 2020

jreback Aug 7, 2020

martindurant Aug 7, 2020

jreback Aug 7, 2020

martindurant Aug 7, 2020

jreback commented Aug 7, 2020

martindurant commented Aug 10, 2020

TomAugspurger left a comment

TomAugspurger commented Aug 10, 2020

		@@ -3126,6 +3156,16 @@ def to_csv(

		.. versionadded:: 1.1.0

		storage_options : dict, optional

Storage options #35381

Storage options #35381

Conversation

martindurant commented Jul 22, 2020 • edited Loading

martindurant commented Jul 23, 2020 • edited Loading

martindurant commented Jul 23, 2020

TomAugspurger commented Jul 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martindurant commented Jul 24, 2020

martindurant commented Aug 6, 2020

martindurant commented Aug 6, 2020

TomAugspurger left a comment

Choose a reason for hiding this comment

TomAugspurger commented Aug 6, 2020

martindurant commented Aug 6, 2020

TomAugspurger commented Aug 6, 2020

martindurant commented Aug 6, 2020

martindurant commented Aug 7, 2020

TomAugspurger commented Aug 7, 2020

martindurant commented Aug 7, 2020

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 7, 2020

martindurant commented Aug 10, 2020

TomAugspurger left a comment

Choose a reason for hiding this comment

TomAugspurger commented Aug 10, 2020

martindurant commented Jul 22, 2020 •

edited

Loading

martindurant commented Jul 23, 2020 •

edited

Loading