-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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/DOC: update pandas-gbq signature and docstring #20564
Conversation
Delegates more of the behavior and documentation for `to_gbq` and `read_gbq` methods to the `pandas-gbq` library. This duplicate documentation was getting out of sync.
Codecov Report
@@ Coverage Diff @@
## master #20564 +/- ##
==========================================
- Coverage 91.82% 91.81% -0.01%
==========================================
Files 153 153
Lines 49256 49265 +9
==========================================
+ Hits 45227 45235 +8
- Misses 4029 4030 +1
Continue to review full report at Codecov.
|
can we just update the signature (of course can have the link as well). It just provides nice tab completeion and doc-strings if its in-line. |
Hello @tswast! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on April 09, 2018 at 08:39 Hours UTC |
@jreback Good point. I've updated the PR to just add the links to the I've built the docs locally and verified that the intersphinx links in the See Also section do render correctly. |
@jreback Was this what you meant? I've reverted the function signatures back to the original (but added |
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.
general philosophy here is simply to update the pandas signature for gbq when they change in impl
it’s not going to be that often
pandas/core/frame.py
Outdated
""" | ||
authentication (eg. Jupyter/IPython notebook on remote host). | ||
kwargs : dict | ||
Arbitrary keyword arguments. |
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.
if these are now official kwargs then add them
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 cb178d9
@@ -1143,33 +1142,57 @@ def to_gbq(self, destination_table, project_id, chunksize=10000, | |||
|
|||
Parameters | |||
---------- | |||
dataframe : DataFrame | |||
DataFrame to be written |
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.
this needs to stay
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.
The validation script was complaining about this one. I think because this is a method of DataFrame
, so there is no dataframe
argument. (I also tried self
and it complained about that, too.)
################################################################################
################################## Validation ##################################
################################################################################
Errors found:
Errors in parameters section
Unknown parameters {'dataframe'}
Parameter "kwargs" description should finish with "."
No returns section found
Missing description for See Also "pandas_gbq.to_gbq" reference
No examples section found
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.
oh don’t worry about that too much
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 think @tswast is correct that this should be removed. The method does not take a dataframe as input (it is writing self
to gbq)
pandas/core/frame.py
Outdated
The main method a user calls to export pandas DataFrame contents to | ||
Google BigQuery table. | ||
def to_gbq( | ||
self, destination_table, project_id, chunksize=10000, |
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 keep this on the previous line as it was? (to keep consistency, we don't use this style anywhere else in the file for def
lines)
pandas/core/frame.py
Outdated
@@ -1144,32 +1143,58 @@ def to_gbq(self, destination_table, project_id, chunksize=10000, | |||
Parameters | |||
---------- | |||
dataframe : DataFrame | |||
DataFrame to be written | |||
DataFrame to be written to Google BigQuery. | |||
destination_table : string |
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.
string -> str
pandas/core/frame.py
Outdated
destination_table : string | ||
Name of table to be written, in the form 'dataset.tablename' | ||
Name of table to be written, in the form 'dataset.tablename'. | ||
project_id : str | ||
Google BigQuery Account project ID. | ||
chunksize : int (default 10000) |
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.
"int (default 10000)" -> "int, default 10000"
(and same comment for other similar cases below)
pandas/core/frame.py
Outdated
""" | ||
authentication (eg. Jupyter/IPython notebook on remote host). | ||
auth_local_webserver : boolean (default False) | ||
Use the [local webserver flow] instead of the [console flow] |
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 think those [..] need a trailing underscore to make them into actual links?
(you can test with python doc/make.py --single DataFrame.to_gbq
)
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 37b2a08
Also, thanks for the test command. Much faster than trying to build the whole docs set!
Also, adjust line breaks per review comments.
I've addressed the comments and CI is now passing again. Is there anything left to do on this PR? |
can u add a note in whatsnew (documentation section is ok) |
Okay. Added to whatsnew. (And made a couple tweaks for recently released of Pandas-GBQ) |
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.
Minor comment
doc/source/whatsnew/v0.23.0.txt
Outdated
@@ -924,6 +924,9 @@ read the `NumFOCUS blogpost`_ recapping the sprint. | |||
to functions, methods and classes. | |||
(:issue:`18941`, :issue:`18948`, :issue:`18973`, :issue:`19017`) | |||
- Added a reference to :func:`DataFrame.assign` in the concatenate section of the merging documentation (:issue:`18665`) | |||
- Updated ``to_gbq`` and ``read_gbq`` documentation to reflect changes from |
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.
maybe "documentation" -> "signature and documentation" (as it is not only a doc change ?)
and move it to "other enhancements" section
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.
quickly addressed it myself
@tswast Thanks! |
Thanks @jorisvandenbossche 🎉 |
can/should we remove "verbose"? |
It hasn't had any effect in pandas-gbq since 2018-04-03. Makes sense to remove in 1.0. |
@tswast great, thanks. how about |
I'm a little more hesitant on |
Delegates more of the behavior and documentation for
to_gbq
andread_gbq
methods to thepandas-gbq
library. This duplicatedocumentation was getting out of sync.
Please include the output of the validation script below between the "```" ticks:
If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.
Validation script gives errors This PR removes documentation which is duplicated in the
pandas-gbq
docs and add intersphinx links to the relevant methods.Checklist for other PRs (remove this part if you are doing a PR for the pandas documentation sprint):
git diff upstream/master -u -- "*.py" | flake8 --diff