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

add java bindings for non-timestamps range window queries [skip ci] #7909

Merged
merged 1 commit into from
May 13, 2021
Merged

add java bindings for non-timestamps range window queries [skip ci] #7909

merged 1 commit into from
May 13, 2021

Conversation

wbo4958
Copy link
Contributor

@wbo4958 wbo4958 commented Apr 8, 2021 •
edited
Loading

This PR is to add java bindings for integral (boolean-exclusive) and timestamps supporting for range window.

@github-actions github-actions bot added CMake CMake build issue conda Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Apr 8, 2021
@wbo4958 wbo4958 changed the title add java bindings for non-timestamps range window queries [WIP] add java bindings for non-timestamps range window queries Apr 8, 2021
@wbo4958 wbo4958 mentioned this pull request Apr 8, 2021
Merged
@wbo4958
Copy link
Contributor Author

wbo4958 commented Apr 8, 2021

This PR depends on #7866

@sameerz sameerz mentioned this pull request Apr 8, 2021
Merged
@mythrocks mythrocks self-requested a review April 29, 2021 17:44
@mythrocks
Copy link
Contributor

mythrocks commented Apr 29, 2021 •
edited
Loading

@wbo4958, at long last, #7866 has been merged. It would be good to rebase against branch-0.20, and see where things stand.

@github-actions github-actions bot removed CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. conda labels Apr 30, 2021
@wbo4958 wbo4958 force-pushed the range-queries-20 branch 2 times, most recently from 1591ddb to 8e48eb3 Compare April 30, 2021 06:14
@wbo4958 wbo4958 changed the title [WIP] add java bindings for non-timestamps range window queries add java bindings for non-timestamps range window queries Apr 30, 2021
@wbo4958 wbo4958 marked this pull request as ready for review April 30, 2021 06:20
@wbo4958 wbo4958 requested a review from a team as a code owner April 30, 2021 06:20
@wbo4958 wbo4958 requested a review from mythrocks April 30, 2021 06:20
@codecov
Copy link

codecov bot commented Apr 30, 2021 •
edited
Loading

Codecov Report

Merging #7909 (7643816) into branch-0.20 (51336df) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 7643816 differs from pull request most recent head c3fea68. Consider uploading reports for the commit c3fea68 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.20    #7909      +/-   ##
===============================================
- Coverage        82.88%   82.88%   -0.01%     
===============================================
  Files              103      104       +1     
  Lines            17668    17899     +231     
===============================================
+ Hits             14645    14836     +191     
- Misses            3023     3063      +40
Impacted Files Coverage Δ
python/cudf/cudf/core/tools/datetimes.py 80.42% <0.00%> (-4.11%) ⬇️
python/cudf/cudf/core/column/decimal.py 90.64% <0.00%> (-2.29%) ⬇️
python/cudf/cudf/core/column/datetime.py 88.03% <0.00%> (-1.88%) ⬇️
python/cudf/cudf/core/column/struct.py 94.73% <0.00%> (-1.56%) ⬇️
python/cudf/cudf/utils/dtypes.py 82.20% <0.00%> (-1.24%) ⬇️
python/dask_cudf/dask_cudf/groupby.py 91.28% <0.00%> (-0.88%) ⬇️
python/cudf/cudf/core/series.py 91.17% <0.00%> (-0.56%) ⬇️
python/cudf/cudf/core/index.py 92.52% <0.00%> (-0.55%) ⬇️
python/cudf/cudf/core/column/column.py 88.20% <0.00%> (-0.44%) ⬇️
python/cudf/cudf/core/column/lists.py 86.98% <0.00%> (-0.43%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2273b6d...c3fea68. Read the comment docs.

@wbo4958
Copy link
Contributor Author

wbo4958 commented May 5, 2021

build

@wbo4958 wbo4958 added 3 - Ready for Review Ready for review by team ! - Release Spark Functionality that helps Spark RAPIDS non-breaking Non-breaking change labels May 5, 2021
mythrocks
mythrocks approved these changes May 7, 2021
View reviewed changes
Copy link
Contributor

@mythrocks mythrocks left a comment

Choose a reason for hiding this comment

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

One minor nitpick, but this looks good to go.

@wbo4958
Copy link
Contributor Author

wbo4958 commented May 7, 2021

One minor nitpick, but this looks good to go.

Thx Mithun, I have changed it according to your comments

@wbo4958
Copy link
Contributor Author

wbo4958 commented May 7, 2021

build

@revans2 revans2 changed the title add java bindings for non-timestamps range window queries add java bindings for non-timestamps range window queries [skip ci] May 7, 2021
@wbo4958
Copy link
Contributor Author

wbo4958 commented May 9, 2021

Hi @revans2 @mythrocks, Since there is lots of conflicts, I rebased to upstream head in only one commit, hope that will not cause any inconvenience when you're reviewing.

thx

@wbo4958
Copy link
Contributor Author

wbo4958 commented May 9, 2021

build

@revans2 revans2 added breaking Breaking change and removed non-breaking Non-breaking change labels May 10, 2021
@revans2
Copy link
Contributor

revans2 commented May 10, 2021

Because this changes the semantics of how the window operations API works I updated this to a breaking change and we should look at getting a fix to the plugin ASAP so we don't leak too much during the tests.

@mythrocks
Copy link
Contributor

@wbo4958, this PR looks good to go. Could I please request for a link to the spark-rapids plugin PR corresponding to this change?

@wbo4958
Copy link
Contributor Author

wbo4958 commented May 13, 2021

Hi @mythrocks , the spark-rapids PR is NVIDIA/spark-rapids#2020

@wbo4958 wbo4958 added breaking Breaking change and removed breaking Breaking change labels May 13, 2021
@wbo4958
Copy link
Contributor Author

wbo4958 commented May 13, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2a169c8 into rapidsai:branch-0.20 May 13, 2021
@wbo4958 wbo4958 deleted the range-queries-20 branch May 13, 2021 22:58
wbo4958 added a commit to wbo4958/cudf that referenced this pull request May 14, 2021
@wbo4958
Revert "add java bindings for non-timestamps range window queries (ra…
7cff611 
…pidsai#7909)"

This reverts commit 2a169c8.
rapids-bot bot pushed a commit that referenced this pull request May 14, 2021
@wbo4958
Revert #7909 add java bindings for non-timestamps range window queries (
304f460 
#8245)

Since it's a breaking PR, and the corresponding spark-rapids PR has some issues. Just revert this cudf PR. Sorry
 
This reverts commit 2a169c8.

Authors:
  - Bobby Wang (https://github.com/wbo4958)

Approvers:
  - Liangcai Li (https://github.com/firestarman)

URL: #8245
@wbo4958 wbo4958 mentioned this pull request May 14, 2021
Merged
rapids-bot bot pushed a commit that referenced this pull request May 14, 2021
@wbo4958
add java bindings for non-timestamps range window queries (#8248)
2d1d4ec 
This PR is to add java bindings for integral (boolean-exclusive) and
timestamps supporting for range window

Please refer to #7909

Authors:
  - Bobby Wang (https://github.com/wbo4958)

Approvers:
  - Jason Lowe (https://github.com/jlowe)

URL: #8248
@mythrocks mythrocks mentioned this pull request May 14, 2021
Merged
@firestarman firestarman mentioned this pull request May 14, 2021
Merged
2 tasks
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 approved these changes

@mythrocks mythrocks mythrocks approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond breaking Breaking change improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wbo4958 @mythrocks @revans2 @vyasr