-
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
feat: limited support of lamdas in Series.apply
#345
Conversation
There is a limited support of simple functions and lambdas which can be | ||
operated directly (without converting into a `remote_function`) on the | ||
BigQuery DataFrames objects. This approach takes advantage of a nuance | ||
in the way BigQuery DataFrames objects are modeled internally and works | ||
only if the function body contains only arithmatic or logical operators. |
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'd rather we rephrase this as "ufunc" emulation support, as defined in the pandas docs. https://pandas.pydata.org/docs/reference/api/pandas.Series.apply.html
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.
Updated, PTAL.
bigframes/series.py
Outdated
) | ||
|
||
if not hasattr(func, "bigframes_remote_function"): | ||
return func(self) |
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.
Let's catch exceptions here and if there's a "message" attribute, append a suggestion to try remote_function, instead.
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, thanks!
bigframes/series.py
Outdated
# supported on a Series. Let's guide the customer to use a | ||
# remote function instead | ||
if hasattr(ex, "message"): | ||
ex.message += "\n{_remote_function_recommendation_message}" |
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 be an f
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.
Thanks for catching, corrected.
|
||
if not hasattr(func, "bigframes_remote_function"): | ||
try: | ||
return func(self) |
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.
Since it may result in incorrect values if this isn't a true vectorized function, let's check for by_row=False
. If by_row="compat"
(default) then raise and suggest either remote_function or by_row=False.
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, PTAL.
bigframes/series.py
Outdated
# It is not a remote function | ||
# Then it must be a vectorized function that applies to the Series | ||
# as a whole | ||
assert ( |
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.
AssertionError is a bit of an odd one to raise. Usually that means some invariant that we never expect to happen has violated. Please raise ValueError
instead.
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, PTAL, thanks.
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.
Love it, thanks!
BEGIN_COMMIT_OVERRIDE
feat: limited support of lambdas in
Series.apply
(#345)END_COMMIT_OVERRIDE
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:
Partially fixes internal issue 295964341 🦕