Skip to content
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

Fix assert_eq related imports #428

Closed

Conversation

galipremsagar
Copy link
Contributor

Recently there has been a change of location of where the testing utils of cudf reside. This PR is adapting to those changes.

reference: rapidsai/cudf#7495

@galipremsagar galipremsagar requested a review from a team as a code owner June 30, 2021 20:24
@github-actions github-actions bot added the Python Related to Python code label Jun 30, 2021
@vyasr
Copy link
Contributor

vyasr commented Jul 2, 2021

@galipremsagar if you don't mind I'll post a new PR tomorrow superseding this one. As discussed on Slack (thread with @shwina and others) non-cudf packages should be avoiding the use of assert_eq.

@vyasr
Copy link
Contributor

vyasr commented Jul 2, 2021

rerun tests

@vyasr
Copy link
Contributor

vyasr commented Jul 2, 2021

In light of rapidsai/cudf#8646, disregard my previous comment for now. Let's get this PR merged to unblock further development. I have the other changeset ready and will push those changes once the cudf testing issue is resolved.

Note that tests on this PR will not pass until the issue causing #427 is fixed, but we'll be stuck without merging at least one of them first and this one is finished.

@cwharris cwharris added bug Something isn't working non-breaking Non-breaking change labels Jul 2, 2021
@cwharris
Copy link
Contributor

cwharris commented Jul 2, 2021

Closing per @vyasr 's comment. Will merge this work into #430 so we can get one PR to fix cuspatial build.

@cwharris cwharris closed this Jul 2, 2021
rapids-bot bot pushed a commit that referenced this pull request Jul 2, 2021
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change Python Related to Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants