-
-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: update pandas-gbq to 0.8.0, adds credentials arg #23662
Conversation
Hello @tswast! Thanks for submitting the PR.
|
reauth=False, if_exists='fail', private_key=None, | ||
auth_local_webserver=False, table_schema=None, location=None, | ||
progress_bar=True, verbose=None): | ||
reauth=False, if_exists='fail', auth_local_webserver=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize private_key
was listed as a breaking change for pandas-gbq but don't believe it was ever deprecated on our end. Is there no other way to go about this than just removing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops sorry I see it was just moved around in the signature. Any reason for doing that? Seems like it would be safer to keep in it's current position
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it to the end because that's where I was told to move verbose
when we deprecated that. Seeing as there are already many keyword arguments preceding it, I don't think there's a risk of people using the position of the argument.
Codecov Report
@@ Coverage Diff @@
## master #23662 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 161 161
Lines 51339 51339
=======================================
Hits 47360 47360
Misses 3979 3979
Continue to review full report at Codecov.
|
pandas/tests/io/test_gbq.py
Outdated
@@ -19,7 +19,7 @@ | |||
api_exceptions = pytest.importorskip("google.api_core.exceptions") | |||
bigquery = pytest.importorskip("google.cloud.bigquery") | |||
service_account = pytest.importorskip("google.oauth2.service_account") | |||
pandas_gbq = pytest.importorskip('pandas_gbq') | |||
pandas_gbq = pytest.importorskip("pandas_gbq", minversion="0.8.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no
you need to support the stated min. version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not bump the minimum version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped. Now it'll error instead of skip if an old version of pandas-gbq is installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm where is this actually erroring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll import at the call to pandas_gbq.read_gbq()
or pandas_gbq.to_gbq()
with a "unexpected 'credentials' argument". There are a couple of versions of pandas-gbq that used **kwargs
in read_gbq()
. Unfortunately, it will just ignore the extra argument in those older versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do those kwargs begin at a certain version? Wondering if we shouldn't have test coverage for those to ensure behavior. Alternately could consider bumping the min version in our requirements files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any explicit versions for pandas-gbq
in any requirements files. https://github.com/pandas-dev/pandas/search?q=pandas-gbq&unscoped_q=pandas-gbq
kwargs for read_gbq
were removed in googleapis/python-bigquery-pandas@e7007a8 which corresponds to pandas-gbq version 0.5.0. Any version before that may silently fail.
doc/source/whatsnew/v0.24.0.txt
Outdated
reflect changes from the `Pandas-GBQ library version 0.8.0 | ||
<https://pandas-gbq.readthedocs.io/en/latest/changelog.html#changelog-0-8-0>`__. | ||
Adds a ``credentials`` argument, which enables the use of any kind of | ||
`google-auth credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add this version change in the dependencies changed section (further down). and update install.rst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in d633cdf
ef24b2c
to
d633cdf
Compare
lgtm. @WillAyd over to you. |
Thanks @tswast |
…fixed * upstream/master: (46 commits) DEPS: bump xlrd min version to 1.0.0 (pandas-dev#23774) BUG: Don't warn if default conflicts with dialect (pandas-dev#23775) BUG: Fixing memory leaks in read_csv (pandas-dev#23072) TST: Extend datetime64 arith tests to array classes, fix several broken cases (pandas-dev#23771) STYLE: Specify bare exceptions in pandas/tests (pandas-dev#23370) ENH: between_time, at_time accept axis parameter (pandas-dev#21799) PERF: Use is_utc check to improve performance of dateutil UTC in DatetimeIndex methods (pandas-dev#23772) CLN: io/formats/html.py: refactor (pandas-dev#22726) API: Make Categorical.searchsorted returns a scalar when supplied a scalar (pandas-dev#23466) TST: Add test case for GH14080 for overflow exception (pandas-dev#23762) BUG: Don't extract header names if none specified (pandas-dev#23703) BUG: Index.str.partition not nan-safe (pandas-dev#23558) (pandas-dev#23618) DEPR: tz_convert in the Timestamp constructor (pandas-dev#23621) PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pandas-dev#23634) TST: Use new arithmetic fixtures, parametrize many more tests (pandas-dev#23757) REF/TST: Add more pytest idiom to parsers tests (pandas-dev#23761) DOC: Add ignore-deprecate argument to validate_docstrings.py (pandas-dev#23650) ENH: update pandas-gbq to 0.8.0, adds credentials arg (pandas-dev#23662) DOC: Improve error message to show correct order (pandas-dev#23652) ENH: Improve error message for empty object array (pandas-dev#23718) ...
git diff upstream/master -u -- "*.py" | flake8 --diff