Frame copy to use __class__ instead of type() #9397
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
madsbk
added
3 - Ready for Review
Codecov Report
@@ Coverage Diff @@
## branch-21.12 #9397 +/- ##
================================================
- Coverage 10.79% 10.74% -0.05%
================================================
Files 116 116
Lines 18869 19081 +212
================================================
+ Hits 2036 2051 +15
- Misses 16833 17030 +197
Continue to review full report at Codecov.
|
galipremsagar
approved these changes
Oct 7, 2021
|
rerun tests |
madsbk
added
5 - Ready to Merge
|
@gpucibot merge |
rapids-bot bot
pushed a commit
to rapidsai/dask-cuda
that referenced
this pull request
Oct 11, 2021
Adding testing of cudf dataframe copy, which fails prior to rapidsai/cudf#9397 #### UPDATE: This PR now handles the jit-unspill failures we have been seeing with cudf v21.12. Some background, when we introduced `ProxyObject`, cudf was using `cudf._lib.table.Table` as the base class for all frame-like objects (dataframes, series, etc). `Table` was implemented in cython so our proxy object had to derive from `Table` in order to support cython functions. The result was to use the class `FrameProxyObject` as a workaround: ```python # In order to support the cuDF API implemented in Cython, we inherit from # `cudf._lib.table.Table`, which is the base class of Index, Series, and # Dataframes in cuDF. # Notice, the order of base classes matters. Since ProxyObject is the first # base class, ProxyObject.__init__() is called on creation, which doesn't # define the Table._data and Table._index attributes. Thus, accessing # FrameProxyObject._data and FrameProxyObject._index is pass-through to # ProxyObejct.__getattr__(), which is what we want. class FrameProxyObject(ProxyObject, cudf._lib.table.Table): pass ``` Now that cudf uses `Frame`, a pure Python class, we don't need this workaround anymore. In fact, using `FrameProxyObject` triggers some infinite recursion errors when used together with v21.12+ cc. @randerzander Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Peter Andreas Entschev (https://github.com/pentschev) URL: #751
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR address the asymmetry in the following line in
Frame.copy():The problem is that
self.__class__might not equaltype(self), which is the case for proxy objects like the one used in Dask-CUDA.Related Dask-CUDA PR: rapidsai/dask-cuda#751
cc. @randerzander