-
Notifications
You must be signed in to change notification settings - Fork 590
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
BUG: Fix cudf import on OmniSciDB backend #2055
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.
can you file an issue upstream for this; importing a module should always raise ImportError and not OSError
| @@ -20,7 +20,7 @@ | |||
|
|
|||
| try: | |||
| from cudf.dataframe.dataframe import DataFrame as GPUDataFrame | |||
| except ImportError: | |||
| except (ImportError, OSError): | |||
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.
is there any way to add a test for this?
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 @jreback for reviewing this PR.
sounds good I will open an issue on cudf repo! thanks!
I think that would be very hard to reproduce this error ib ibis CI, as cudf depends on CUDA .. it will be hard to reproduce this environment on ibis CI.
as it is a very small fix, changing CI just for that would be not a good approach (also I am not sure that we can reproduce that on CI)
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 @xmnlab |
Fix #2053