-
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
Enable pd.Timestamp
objects to be picklable when cudf.pandas
is active
#14474
Conversation
…14388) Closes rapidsai#14384. `x.startswith(y)` is not a good enough check for if `x` is a subdirectory of `y`. It causes `pandasai` to be reported as a sub-package of `pandas`. Authors: - Ashwin Srinath (https://github.com/shwina) Approvers: - https://github.com/brandon-b-miller URL: rapidsai#14388
return _unpickle_timestamp, (pickled_args,) | ||
|
||
|
||
def _unpickle_timedelta(pickled_args): |
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.
How is _unpickle_timedelta
different from _unpickle_timestamp
? Similarly for _reduce_*
. They look identical to me. Do we need this duplication or can we simplify things?
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.
Agreed, this looks like it can be deduplicated.
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.
Right you are -- they are the same function and I've deduplicated
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.
One question about duplication -- otherwise LGTM. Waiting to approve until we can discuss that question.
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.
Approving assuming Bradley's question about duplication is addressed.
return _unpickle_timestamp, (pickled_args,) | ||
|
||
|
||
def _unpickle_timedelta(pickled_args): |
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.
Agreed, this looks like it can be deduplicated.
/merge |
Closes #14471.
This PR makes
Timestamp
objects picklable by registering a custom reducer forpd.Timestamp
objects whencudf.pandas
is active.