-
Notifications
You must be signed in to change notification settings - Fork 63
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
| 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 🦕