Skip to content
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

#34640: CLN: remove 'private_key' and 'verbose' from gbq #34654

Merged
merged 21 commits into from
Sep 24, 2020

Conversation

parkdj1
Copy link
Contributor

@parkdj1 parkdj1 commented Jun 8, 2020

Hello, I am new to contributing to open source, so thank you in advance for any corrections and suggestions. Please let me know if I need to do anything else for this ticket!

I removed the code containing both 'private_key' and 'verbose' from the gbq.py file.

@WillAyd
Copy link
Member

WillAyd commented Jun 9, 2020

@tswast

@charlesdong1991
Copy link
Member

since those two kwargs weren't removed along with #30200 , also cc @jbrockmendel , just in case this was made on purpose by him.

@jreback jreback added the Clean label Jun 9, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can you add a note in other api changes section for 1.1 that these keywords were removed, also please merge master and ping on green.

@@ -379,7 +379,7 @@ Other API changes
- ``loc`` lookups with an object-dtype :class:`Index` and an integer key will now raise ``KeyError`` instead of ``TypeError`` when key is missing (:issue:`31905`)
- Using a :func:`pandas.api.indexers.BaseIndexer` with ``count``, ``min``, ``max``, ``median``, ``skew``, ``cov``, ``corr`` will now return correct results for any monotonic :func:`pandas.api.indexers.BaseIndexer` descendant (:issue:`32865`)
- Added a :func:`pandas.api.indexers.FixedForwardWindowIndexer` class to support forward-looking windows during ``rolling`` operations.
-
- Removed ``private_key`` and ``verbose`` from gbq (:issue:`34654`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you co-locate this with the other issue where we removed a keyword from gbq; also pls use the references to read_gbq (see the other issue)

@charlesdong1991
Copy link
Member

charlesdong1991 commented Jun 17, 2020

@parkdj1

hi, you need to rebase and resolve the conflicts in the two files listed below, otherwise, your commit won't get through and make CI run.

@alimcmaster1
Copy link
Member

@parkdj1 - Thanks for the PR, can you address the test failures and fix up the commits seems to be a few unrelated changes.

@pep8speaks
Copy link

pep8speaks commented Jun 27, 2020

Hello @parkdj1! 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-09-22 22:31:42 UTC

@parkdj1
Copy link
Contributor Author

parkdj1 commented Jun 29, 2020

Hello @parkdj1! 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-06-29 00:35:21 UTC

Awesome, thank you!

@alimcmaster1
Copy link
Member

alimcmaster1 commented Aug 21, 2020

@parkdj1 is this still active? Ref my comments above: #34654 (comment)

Also whatsnew will be 1.2 now.

Thanks

Copy link
Member

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

since needs to resolve conflict, a minor comment ^^

doc/source/whatsnew/v1.2.0.rst Outdated Show resolved Hide resolved
@jreback jreback added this to the 1.2 milestone Sep 13, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

minor comments, pls merge master and ping on green. I don't think we have any remaining tests for these right?

doc/source/whatsnew/v1.2.0.rst Outdated Show resolved Hide resolved
doc/source/whatsnew/v1.2.0.rst Show resolved Hide resolved
@jreback jreback merged commit f34a56b into pandas-dev:master Sep 24, 2020
@jreback
Copy link
Contributor

jreback commented Sep 24, 2020

thanks @parkdj1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLN: remove private_key and verbose from gbq
7 participants