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

Refactor spatial related functions under spatial package #656

Merged
merged 23 commits into from
Aug 23, 2022
Merged

Refactor spatial related functions under spatial package #656

merged 23 commits into from
Aug 23, 2022

Conversation

isVoid
Copy link
Contributor

@isVoid isVoid commented Aug 18, 2022 •
edited
Loading

This PR contributes to #644, creates a spatial package for spatial related functions. Divided into six sub categories: distance, bounding, indexing, join, projection, filtering.

Also contributes to #525, #599

@isVoid isVoid requested a review from a team as a code owner August 18, 2022 05:36
@isVoid isVoid requested a review from cwharris August 18, 2022 05:36
@github-actions github-actions bot added the Python Related to Python code label Aug 18, 2022
@isVoid isVoid marked this pull request as draft August 18, 2022 05:36
@isVoid isVoid added improvement Improvement / enhancement to an existing function documentation non-breaking Non-breaking change labels Aug 18, 2022
@isVoid isVoid removed the request for review from cwharris August 18, 2022 05:37
@thomcom
Copy link
Contributor

thomcom commented Aug 18, 2022

Nice

@isVoid
Copy link
Contributor Author

isVoid commented Aug 19, 2022

Explaining for recent 2 revert changes: this branch originally based off from #645, but the changes made in this PR are very much independent. I reverted all commits included in #645 to remove the dependency.

@isVoid isVoid marked this pull request as ready for review August 19, 2022 18:57
thomcom
thomcom approved these changes Aug 19, 2022
View reviewed changes
Copy link
Contributor

@thomcom thomcom left a comment

Choose a reason for hiding this comment

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

Great work

harrism
harrism reviewed Aug 22, 2022
View reviewed changes
docs/source/api_docs/index.rst Show resolved Hide resolved
docs/source/api_docs/spatial.rst Outdated Show resolved Hide resolved
@isVoid isVoid requested a review from harrism August 23, 2022 03:58
@isVoid
Copy link
Contributor Author

isVoid commented Aug 23, 2022

Note the PR is updated per changes mentioned in #644 (comment)

harrism
harrism approved these changes Aug 23, 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.

Looks good, thanks!

@isVoid
Copy link
Contributor Author

isVoid commented Aug 23, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit eba9f99 into rapidsai:branch-22.10 Aug 23, 2022
@isVoid isVoid mentioned this pull request Aug 23, 2022
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 24, 2022
@isVoid
Rename Test Folders to Align with Module Names (#661)
df3a163 
#656 missed refactoring the test files to align with the names in the spatial module. Though it's not an explicit requirement in cuDF python, since the API surface of cuspatial is still manageable, we can try to enforce a similar folder structure in tests as the core module.

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

Approvers:
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #661
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@thomcom thomcom thomcom approved these changes

@trxcllnt trxcllnt Awaiting requested review from trxcllnt

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to Python code
Projects
cuSpatial
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@isVoid @thomcom @harrism