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

[FEA]: Remove dependency on GDAL in libcuspatial point-in-polygon tests #958

Closed
harrism opened this issue Feb 27, 2023 · 0 comments · Fixed by #974
Closed

[FEA]: Remove dependency on GDAL in libcuspatial point-in-polygon tests #958

harrism opened this issue Feb 27, 2023 · 0 comments · Fixed by #974
Assignees
Labels
feature request New feature or request libcuspatial Relates to the cuSpatial C++ library tests Relating to tests and test automation

Comments

@harrism
Copy link
Member

harrism commented Feb 27, 2023

Is this a new feature, an improvement, or a change to existing functionality?

Improvement

How would you describe the priority of this feature request

Medium

Please provide a clear description of problem you would like to solve.

We have removed GDAL as a dependency (#932) in all but one C++ unit test: https://github.com/rapidsai/cuspatial/blob/branch-23.04/cpp/tests/join/point_in_polygon_test_large.cpp

GDAL is used for its contains functionality to verify our quadtree spatial join. This test only has a few polygons, so we could use our all-pairs point-in-polygon API which is limited to 31 polygons for verification, and drop the GDAL dependency completely, which would help with building PIP wheel unit tests.

Describe any alternatives you have considered

No response

Additional context

No response

@harrism harrism added feature request New feature or request Needs Triage Need team to review and classify labels Feb 27, 2023
@harrism harrism self-assigned this Feb 27, 2023
@harrism harrism added libcuspatial Relates to the cuSpatial C++ library and removed Needs Triage Need team to review and classify labels Feb 27, 2023
@harrism harrism added the tests Relating to tests and test automation label Feb 27, 2023
@harrism harrism moved this from Todo to Review in cuSpatial Mar 7, 2023
@rapids-bot rapids-bot bot closed this as completed in #974 Mar 13, 2023
rapids-bot bot pushed a commit that referenced this issue Mar 13, 2023
Closes #958.  Replaces usage of GDAL `contains` in the quadtree point-in-polygon tests with a call to a different cuSpatial point-in-polygon function. This allows us to remove all current dependencies on GDAL in cuSpatial.

Depends on #973. This PR currently updates some expectations to make tests pass that are fixed in that PR.

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

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - H. Thomson Comer (https://github.com/thomcom)
  - Paul Taylor (https://github.com/trxcllnt)
  - Jordan Jacobelli (https://github.com/jjacobelli)

URL: #974
@github-project-automation github-project-automation bot moved this from Review to Done in cuSpatial Mar 13, 2023
trxcllnt pushed a commit to trxcllnt/cuspatial that referenced this issue Mar 15, 2023
Closes rapidsai#958.  Replaces usage of GDAL `contains` in the quadtree point-in-polygon tests with a call to a different cuSpatial point-in-polygon function. This allows us to remove all current dependencies on GDAL in cuSpatial.

Depends on rapidsai#973. This PR currently updates some expectations to make tests pass that are fixed in that PR.

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

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - H. Thomson Comer (https://github.com/thomcom)
  - Paul Taylor (https://github.com/trxcllnt)
  - Jordan Jacobelli (https://github.com/jjacobelli)

URL: rapidsai#974
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request libcuspatial Relates to the cuSpatial C++ library tests Relating to tests and test automation
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant