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

[REVIEW] Series median() with null support #2905

Merged
merged 17 commits into from
Nov 19, 2019
Merged

[REVIEW] Series median() with null support #2905

merged 17 commits into from
Nov 19, 2019

Conversation

karthikeyann
Copy link
Contributor

@karthikeyann karthikeyann commented Oct 1, 2019 •
edited
Loading

Closes #1085

  • Quantiles with null support
  • Series Median
  • Series Median skipna feature

@harrism harrism added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. labels Oct 2, 2019
@codecov
Copy link

codecov bot commented Oct 4, 2019 •
edited
Loading

Codecov Report

Merging #2905 into branch-0.11 will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##           branch-0.11   #2905      +/-   ##
==============================================
+ Coverage         87.3%   87.3%   +<.01%     
==============================================
  Files               49      49              
  Lines             9214    9218       +4     
==============================================
+ Hits              8044    8048       +4     
  Misses            1170    1170
Impacted Files Coverage Δ
python/cudf/cudf/core/series.py 93.84% <100%> (+0.02%) ⬆️

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 d640dd8...beb9539. Read the comment docs.

@karthikeyann karthikeyann marked this pull request as ready for review October 4, 2019 21:39
@karthikeyann karthikeyann requested review from a team as code owners October 4, 2019 21:39
@karthikeyann karthikeyann changed the title [WIP] Series median() with null support [REVIEW] Series median() with null support Oct 4, 2019
jrhemstad
jrhemstad requested changes Oct 7, 2019
View reviewed changes
cpp/src/quantiles/quantiles.cu Outdated Show resolved Hide resolved
cpp/src/quantiles/quantiles.cu Outdated Show resolved Hide resolved
cpp/src/quantiles/quantiles.cu Outdated Show resolved Hide resolved
harrism
harrism approved these changes Nov 17, 2019
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@karthikeyann @harrism
Update CHANGELOG.md
dc1f1dc 
Co-Authored-By: Mark Harris <mharris@nvidia.com>
@karthikeyann
Copy link
Contributor Author

Why merging is blocked?

@harrism
Copy link
Member

harrism commented Nov 19, 2019

Because @jrhemstad requested changes.

@harrism
Copy link
Member

harrism commented Nov 19, 2019 •
edited
Loading

Also presumably this is going to conflict with the port of quantiles to libcudf++ #3389? Shouldn't the bug be fixed in the libcudf++ version instead?

@karthikeyann
Copy link
Contributor Author

Also presumably this is going to conflict with the port of quantiles to libcudf++ #3389? Shouldn't the bug be fixed in the libcudf++ version instead?

That PR #3389 is merged. I pulled branch-0.11 changes and merged. No conflicts.

@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond and removed 2 - In Progress Currently a work in progress labels Nov 19, 2019
@karthikeyann karthikeyann merged commit 6c8ce65 into rapidsai:branch-0.11 Nov 19, 2019
@karthikeyann karthikeyann removed 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond labels Nov 19, 2019
@jangorecki jangorecki mentioned this pull request Nov 21, 2019
Closed
@jangorecki jangorecki mentioned this pull request Jan 9, 2020
Closed
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

@kkraus14 kkraus14 kkraus14 approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

@beckernick beckernick Awaiting requested review from beckernick

Assignees
No one assigned
Labels
libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Series.median()
4 participants
@karthikeyann @harrism @kkraus14 @jrhemstad