Skip to content

[REVIEW] Update to use __reduce_ex__ in CumlArray to override cudf.Buffer#2228

Merged
dantegd merged 6 commits intorapidsai:branch-0.14from
dantegd:fix-cuary-pickleser
May 11, 2020
Merged

[REVIEW] Update to use __reduce_ex__ in CumlArray to override cudf.Buffer#2228
dantegd merged 6 commits intorapidsai:branch-0.14from
dantegd:fix-cuary-pickleser

Conversation

@dantegd
Copy link
Member

@dantegd dantegd commented May 10, 2020

cuDF's PR rapidsai/cudf#5132 to optimize __reduce_ex__ in cudf.Buffer broke pickling and serialization in CumlArray, since there only __reduce__ was being implemented. This PR addresses that by adding an explicit __reduce_ex__ in addition to __reduce__.

@dantegd dantegd added 3 - Ready for Review Ready for review by team gpuCI gpuCI issue Cython / Python Cython or Python issue labels May 10, 2020
@dantegd dantegd requested a review from a team as a code owner May 10, 2020 14:49
@dantegd dantegd changed the title [REVIEW[ Update to use __reduce_ex__ in CumlArray to override cudf.Buffer [REVIEW] Update to use __reduce_ex__ in CumlArray to override cudf.Buffer May 10, 2020
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dantegd
Copy link
Member Author

dantegd commented May 10, 2020

rerun tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would just stick with __reduce_ex__ and drop __reduce__. Pickle just needs one of them implemented.

Also as a note __reduce__ doesn’t take any additional arguments.

@dantegd dantegd merged commit 9aee75d into rapidsai:branch-0.14 May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team Cython / Python Cython or Python issue gpuCI gpuCI issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments