Skip to content

Conversation

@cdknox
Copy link
Contributor

@cdknox cdknox commented Nov 20, 2020

@jreback
Copy link
Contributor

jreback commented Nov 21, 2020

cc @twoertwein

@jreback jreback added the IO CSV read_csv, to_csv label Nov 21, 2020
Copy link
Member

@twoertwein twoertwein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! The only part missing is the documentation update in pandas/core/shared_docs.py and making sure that all functions that take storage_options work as expected with it. imho I would only add a new test for parquet, the other functions forward everything directly to get_handle.

@jreback
Copy link
Contributor

jreback commented Nov 22, 2020

we have tons of fixtures eg s3_resources

pls try to not reinvent the wheel

if u must mock then use monkeypatch

prefer not to mock at all

@pep8speaks
Copy link

pep8speaks commented Dec 3, 2020

Hello @cdknox! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

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

Comment last updated at 2020-12-13 23:17:56 UTC

@cdknox
Copy link
Contributor Author

cdknox commented Dec 12, 2020

I'm sure folks are busy; I just wanted to make sure no one is waiting on me for anything. I think I've addressed the comments so far, but if I'm missing something or I need to do something else please let me know!

@jreback jreback added this to the 1.3 milestone Dec 14, 2020
@jreback jreback merged commit a226941 into pandas-dev:master Dec 15, 2020
@jreback
Copy link
Contributor

jreback commented Dec 15, 2020

thanks @cdknox very nice

@cdknox
Copy link
Contributor Author

cdknox commented Dec 15, 2020

Glad to help as a long time pandas user! Just double checking, we're good without the change then from storage_options={"User-Agent": "custom_user_agent"} to storage_options={"headers": {"User-Agent": "custom_user_agent"}}? I'm good with it either way

luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021
path,
engine: str = "auto",
columns=None,
storage_options: StorageOptions = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new parameter should maybe be added after use_nullable_dtypes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

IO CSV read_csv, to_csv

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: Change Pandas User-Agent and add possibility to set custom http_headers to pd.read_* functions

6 participants