- 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()
tswast left a comment
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
tswast left a comment
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 🦕