-
Notifications
You must be signed in to change notification settings - Fork 902
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
Raise errors on specific types of fallback in cudf.pandas
#17268
base: branch-24.12
Are you sure you want to change the base?
Conversation
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 will be super helpful, thanks Matt! I have some suggestions to clean things up and make the reporting a little more informative.
@@ -881,12 +883,36 @@ def _assert_fast_slow_eq(left, right): | |||
assert_eq(left, right) | |||
|
|||
|
|||
class ProxyFallbackError(Exception): | |||
class FallbackError(Exception): |
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 debugging purposes it might be helpful to store func.__name__
from _fast_slow_function_call
into the exception.
"""Raised when fallback occurs""" | ||
|
||
pass | ||
|
||
|
||
class OOMFallbackError(FallbackError): | ||
"""Warns when cuDF produces a MemoryError or an rmm.RMMError""" |
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.
All the docstrings should say "Raises" instead of Warns".
raise OOMFallbackError( | ||
"Out of Memory Error. Falling back to the slow path. " | ||
f"The exception was {err}." | ||
) |
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 would decorate all of these raises with a from err
so that we get the full exception chain (see the docs on the from
clause if you're not familiar).
"mock_mean, err, match_str", | ||
[ | ||
( | ||
mock_mean_memory_error, |
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 these are all single-use functions I'd suggest using lambdas instead.
Description
Closes #14975
Checklist