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

Create CommonGeometryColumnFixture for Common Geometry Column Reuse #1104

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Apr 27, 2023

Description

This PR adds a CommonGeometryColumnFixture manages the life cycle of some commonly used geometry columns that can be shared across different test suites. Currently, only empty geometry columns are added.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@isVoid isVoid requested a review from a team as a code owner April 27, 2023 22:49
@isVoid isVoid requested review from trxcllnt and harrism April 27, 2023 22:49
@isVoid isVoid self-assigned this Apr 27, 2023
@github-actions github-actions bot added the libcuspatial Relates to the cuSpatial C++ library label Apr 27, 2023
@isVoid isVoid requested a review from a team as a code owner April 27, 2023 22:54
@github-actions github-actions bot added the cmake Related to CMake code or build configuration label Apr 27, 2023
@isVoid isVoid changed the title Moving incorrectly placed linestring_polygon_distance_test.cpp to distance/ Move incorrectly placed linestring_polygon_distance_test.cpp to distance/ Apr 27, 2023
@isVoid isVoid added non-breaking Non-breaking change bug Something isn't working labels Apr 27, 2023
@harrism
Copy link
Member

harrism commented May 3, 2023

So some of this was done by #1097 but it looks like this also refactors some test fixture code. So perhaps the title of this PR should change?

Also, I realized that in #1097 I missed moving this file. Could you fix it in this PR? https://github.com/rapidsai/cuspatial/blob/branch-23.06/cpp/include/cuspatial/distance/polygon_distance.hpp The function in that file should move into cpp/include/cuspatial/distance.hpp Done in #1115

…into improvement/move_linestring_polygon_distance
@isVoid isVoid changed the title Move incorrectly placed linestring_polygon_distance_test.cpp to distance/ Create CommonGeometryColumnFixture for Common Geometry Column Reuse May 3, 2023
@isVoid
Copy link
Contributor Author

isVoid commented May 3, 2023

Let's hold on reviewing this PR, thinking on a better design for the geometry fixtures.

@isVoid isVoid marked this pull request as draft May 3, 2023 22:46
@isVoid
Copy link
Contributor Author

isVoid commented May 4, 2023

Supersedes by #1124

@isVoid isVoid closed this May 4, 2023
rapids-bot bot pushed a commit that referenced this pull request May 16, 2023
This PR makes all ST_Distance API conforms to a homogenous API format and documentation. This also greatly simplifies the implementation of each of the column APIs. Closes #1123 

This PR also introduces several `GeometryColumnFixtures` that manages the life time of a few commonly used geometry columns and use them across the tests of these APIs. Supersedes #1104 

This PR also fixes several bugs in the computation kernels when the input is empty.

Authors:
  - Michael Wang (https://github.com/isVoid)
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Mark Harris (https://github.com/harrism)

URL: #1124
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cmake Related to CMake code or build configuration libcuspatial Relates to the cuSpatial C++ library non-breaking Non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants