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.

Sign up for GitHub

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

Jump to bottom

[FIX] Update quadtree spatial join docstrings #797

Merged
merged 9 commits into from
Nov 22, 2022
Merged

[FIX] Update quadtree spatial join docstrings #797

merged 9 commits into from
Nov 22, 2022

Conversation

trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Nov 14, 2022 •
edited by harrism
Loading

@trxcllnt trxcllnt requested review from a team as code owners November 14, 2022 22:05
@github-actions github-actions bot added libcuspatial Relates to the cuSpatial C++ library Python Related to Python code labels Nov 14, 2022
@trxcllnt trxcllnt added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Python Related to Python code libcuspatial Relates to the cuSpatial C++ library labels Nov 14, 2022
@trxcllnt trxcllnt requested review from isVoid, thomcom and harrism and removed request for jrhemstad, thomcom and cwharris November 14, 2022 22:07
@github-actions github-actions bot added libcuspatial Relates to the cuSpatial C++ library Python Related to Python code labels Nov 14, 2022
@harrism
Copy link
Member

harrism commented Nov 14, 2022

@trxcllnt since #733 is a long discussion and recommends a different fix rather than just documentation, can you please comment in the description on how specifically this PR fixes that issue?

@trxcllnt
Copy link
Contributor Author

Commit 37ca193 fixes #733 by updating the docstring to indicate the resulting indices are offsets into the input point_indices, and not the original point_x and point_y columns.

@harrism harrism mentioned this pull request Nov 15, 2022
Closed
harrism
harrism requested changes Nov 15, 2022
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes. Some questions.

python/cuspatial/cuspatial/core/spatial/indexing.py Outdated Show resolved Hide resolved
python/cuspatial/cuspatial/core/spatial/indexing.py Show resolved Hide resolved
python/cuspatial/cuspatial/core/spatial/join.py Outdated Show resolved Hide resolved
@trxcllnt trxcllnt added breaking Breaking change and removed non-breaking Non-breaking change labels Nov 21, 2022
@harrism harrism mentioned this pull request Nov 21, 2022
Closed
@trxcllnt
Copy link
Contributor Author

rerun tests

@trxcllnt
Copy link
Contributor Author

rerun tests

@trxcllnt
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit da7d3a4 into rapidsai:branch-22.12 Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomcom thomcom thomcom approved these changes

@harrism harrism harrism approved these changes

@isVoid isVoid Awaiting requested review from isVoid

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team breaking Breaking change improvement Improvement / enhancement to an existing function libcuspatial Relates to the cuSpatial C++ library Python Related to Python code
Projects
cuSpatial
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] quadtree_point_in_polygon.cu, bug and fix [DOC/QST] Scale parameter in quadtree methods
3 participants
@trxcllnt @harrism @thomcom