-
Notifications
You must be signed in to change notification settings - Fork 42
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
fix: fix read_gbq_function issue in dataframe apply method #1174
base: main
Are you sure you want to change the base?
Conversation
@@ -3674,7 +3674,11 @@ def apply(self, func, *, axis=0, args: typing.Tuple = (), **kwargs): | |||
return result_series | |||
|
|||
# Per-column apply | |||
results = {name: func(col, *args, **kwargs) for name, col in self.items()} | |||
if hasattr(func, "bigframes_remote_function"): |
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.
@TrevorBergeron could you please review if this element-wise proxy for series-wise application doesn't have any blind sight?
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.
Isn't dataframe.apply
for column or series-wise application? Why do we need to support applying as an element-wise operator? isn't this what dataframe.map
is for?
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.
For scalar ops the end result is the same. I think we don't lose anything by being lax for user convenience. Note that this API has other non-pandas-like extended behavior
python-bigquery-dataframes/bigframes/dataframe.py
Line 3933 in e0a8288
# This is a special case where we are providing not-pandas-like |
|
||
func_ref = session.read_gbq_function("bqutil.fn.cw_lower_case_ascii_only") | ||
bf_result = bdf.apply(func_ref).to_pandas() | ||
# The str.lower() method has the same functionality as func_ref. |
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 is true for ASCII only, we should say that in the comment
See the description/example from b/379750234.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕