-
Notifications
You must be signed in to change notification settings - Fork 915
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 metadata after implicit array conversion from Dask cuDF #16842
Fix metadata after implicit array conversion from Dask cuDF #16842
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.
I think this makes sense
@get_collection_type.register(cupy.ndarray) | ||
def get_collection_type_cupy_array(_): | ||
return _create_array_collection_with_meta |
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.
@get_collection_type.register(cupy.ndarray) | |
def get_collection_type_cupy_array(_): | |
return _create_array_collection_with_meta | |
get_collection_type.register(cupy.ndarray)(_create_array_collection_with_meta) |
?
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 doesn't quite work. I can change to the following if you don't like the decorator pattern for this case:
get_collection_type.register(
cupy.ndarray,
lambda x: _create_array_collection_with_meta,
)
The "trick" is that we need to register a function that simply returns a function that accepts an Expr argument.
@get_collection_type.register(spmatrix) | ||
def get_collection_type_csr_matrix(_): | ||
return _create_array_collection_with_meta |
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.
If the suggestion above works, the same thing here?
/merge |
Description
Temporary workaround for dask/dask#11017 in Dask cuDF (when query-planning is enabled).
I will try to move this fix upstream soon. However, the next dask release will probably not be used by 24.10, and it's still unclear whether the same fix works for all CPU cases.
Checklist