-
Notifications
You must be signed in to change notification settings - Fork 921
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
Remove cudf._lib.csv in favor in inlining pylibcudf #17485
Conversation
def _get_plc_data_type_from_dtype(dtype) -> plc.DataType: | ||
# TODO: Remove this work-around Dictionary types | ||
# in libcudf are fully mapped to categorical columns: | ||
# https://github.com/rapidsai/cudf/issues/3960 |
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.
Do you know what this TODO entails? It looks like issue linked is closed.
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'm not certain; it looks like this description changed in #12571.
I'm guessing that it may have to do with the CSV reader round tripping when cudf Python starts with a categorical dtype? @galipremsagar
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.
Nonetheless, I suppose this can be addressed in a follow-up
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.
Ok, addressing this in a follow up is fine with me.
/merge |
Description
Contributes to #17317
Checklist