Fix build from changes in rapidsai/cudf#8528 #427
Conversation
cwharris
added
bug
| out._index = index | ||
| if columns is not None: | ||
| out.columns = columns | ||
| return out |
There was a problem hiding this comment.
This seems identical to DataFrame._from_data, so maybe we just not override it at all?
There was a problem hiding this comment.
Yeah, still playing around trying to fix the last CI error, but this probably just goes away.
There was a problem hiding this comment.
Looking at the constructor of this class, it seems like this method probably needs some logic like
if not isinstance(next(iter(data.values())), GeoColumn):
# Convert cudf Columns to GeoColumns in `data`
return super()._from_data(data, index, columns)
unless this method is never called with a dict of GeoColumns.
| @classmethod | ||
| def _from_data(cls, new_data, name=None, index=False): |
There was a problem hiding this comment.
I was very confused by the original description of this PR since _from_data was always a classmethod :) I see the edit, but perhaps the change you're referring to happened on a different PR of mine? AFAICT the one you're linking to didn't modify the relevant APIs at all.
| out._index = index | ||
| if columns is not None: | ||
| out.columns = columns | ||
| return out |
There was a problem hiding this comment.
Looking at the constructor of this class, it seems like this method probably needs some logic like
if not isinstance(next(iter(data.values())), GeoColumn):
# Convert cudf Columns to GeoColumns in `data`
return super()._from_data(data, index, columns)
unless this method is never called with a dict of GeoColumns.
|
I'm aiming to post a fix for this issue tomorrow. |
…ing (#430) This PR contains three distinct changes required to get cuspatial builds working and tests passing again: 1. RMM switched to rapids-cmake (rapidsai/rmm#800), which requires CMake 3.20.1, so this PR includes the required updates for that. 2. The Arrow upgrade in cudf also moved the location of testing utilities (rapidsai/cudf#7495). Long term cuspatial needs to move away from use of the testing utilities, which are not part of cudf's public API, but we are currently blocked by rapidsai/cudf#8646, so this PR just imports the internal `assert_eq` method as a stopgap to get tests passing. 3. The changes in rapidsai/cudf#8373 altered the way that metadata was propagated to libcudf outputs from previously existing cuDF Python objects. The new code paths require cuspatial to override metadata copying at the GeoDataFrame rather than the GeoColumn level in order to ensure that information about column types is lost in the libcudf round trip and the metadata copying functions are now called on the output DataFrame rather than the input one. This PR supersedes #427, #428, and #429, all of which can now be closed. Authors: - Vyas Ramasubramani (https://github.com/vyasr) - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Christopher Harris (https://github.com/cwharris) URL: #430
rapidsai/cudf#8528 introduced some changes which broke the cuspatial build.
specificallycudf no longer accidentally treatscudf.DataFrame._from_datawas change to a@classmethod_from_dataas an instance method, which is bad behavior cuspatial was depending on. To fix this, adjustments need to be made tocuspatial.GeoDataFrameand some associated utilities.PR is WIP, but so far fixes all but one failing test.