Fix copying dtype metadata after calling libcudf functions #7271
Conversation
…ix-dtype-metadata-preservation
python/cudf/cudf/core/frame.py
Outdated
|
|
||
| if include_index and self._index is not None: | ||
| assert other._index is not None |
There was a problem hiding this comment.
Should we throw with an error message here instead of assert?
There was a problem hiding this comment.
This isn't meant to ever propagate up to the user. The assert is used here to tell MyPy that other._index is of type Index (and not Optional[Index]):
https://mypy.readthedocs.io/en/stable/kinds_of_types.html#optional-types-and-the-none-type
There was a problem hiding this comment.
cc: @kkraus14 for visiblity as we were just discussing this
There was a problem hiding this comment.
I think we can do better here by checking if other is a DataFrameOrSeries instead. Looking into it
There was a problem hiding this comment.
Just used a conditional instead of using an assertion here. Along the way, realized we could simplify this function further.
kkraus14
added
the
5 - Ready to Merge
|
rerun tests |
1 similar comment
|
rerun tests |
…ix-dtype-metadata-preservation
|
@gpucibot merge |
Codecov Report
@@ Coverage Diff @@
## branch-0.18 #7271 +/- ##
===============================================
+ Coverage 82.09% 82.21% +0.12%
===============================================
Files 97 100 +3
Lines 16474 16955 +481
===============================================
+ Hits 13524 13940 +416
- Misses 2950 3015 +65
Continue to review full report at Codecov.
|
Fixes #7249
Copies dtype metadata after calling
ColumnBase.copy(). Moves logic for copying dtype metadata after calling libcudf functions fromFrametoColumnBase.