-
Notifications
You must be signed in to change notification settings - Fork 48
feat: warn the deprecated max_download_size
, random_state
and sampling_method
parameters in (DataFrame|Series).to_pandas()
#1573
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
Conversation
d13bd11
to
74be48d
Compare
random_state
and sampling_method
parameters and in (DataFrame|Series).to_pandas()
random_state
and sampling_method
parameters and in (DataFrame|Series).to_pandas()random_state
and sampling_method
parameters in (DataFrame|Series).to_pandas()
74be48d
to
d41fea1
Compare
d41fea1
to
18661bb
Compare
d60123c
to
9711b83
Compare
random_state
and sampling_method
parameters in (DataFrame|Series).to_pandas()
max_download_size
, random_state
and sampling_method
parameters in (DataFrame|Series).to_pandas()
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.
Looks like theres some typos in the docstrings. Please fix.
bigframes/dataframe.py
Outdated
@@ -1636,17 +1636,27 @@ def to_pandas( | |||
|
|||
Args: | |||
max_download_size (int, default None): | |||
.. deprecated:: 2.0.0 | |||
`max_download_size` parameter is deprecated. Please use `to_pandas_batch()` method |
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.
Nit: RST uses two backticks for code format. One is italics.
`max_download_size` parameter is deprecated. Please use `to_pandas_batch()` method | |
``max_download_size`` parameter is deprecated. Please use ``to_pandas_batches()`` method |
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.
Good catches. Fixed!
bigframes/dataframe.py
Outdated
if max_download_size is not None: | ||
msg = bfe.format_message( | ||
"DEPRECATED: The `max_download_size` parameters for `DataFrame.to_pandas()` " | ||
"are deprecated and will be removed soon. Please use `DataFrame.to_pandas_batch()`." |
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.
"are deprecated and will be removed soon. Please use `DataFrame.to_pandas_batch()`." | |
"are deprecated and will be removed soon. Please use ``DataFrame.to_pandas_batches()``." |
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.
Good catches. Fixed!
f64c060
to
ea82a5b
Compare
38a085b
to
c1d379d
Compare
c1d379d
to
626d432
Compare
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.
Thanks!
I added a commit to use FutureWarning instead of UserWarning. See: https://docs.python.org/3/library/exceptions.html#FutureWarning
Per https://stackoverflow.com/a/55378483/101923 both DeprecationWarning and PendingDeprecationWarning are ignored by default, so FutureWarning is the most appropriate for bigframes which is used in notebooks and won't necessarily have pytest test code or similar where the deprecation warnings would be seen.
83fb83f
to
a22a470
Compare
9f72eb5
to
a22a470
Compare
Fixes internal issue 391676515 🦕