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

Implement Series.datetime.floor #9571

Merged
merged 66 commits into from
Nov 17, 2021

Conversation

skirui-source
Copy link
Contributor

@skirui-source skirui-source commented Oct 30, 2021

Fixes: #7102
Replaces: #9488

@skirui-source skirui-source added feature request New feature or request 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. labels Oct 30, 2021
@codecov
Copy link

codecov bot commented Oct 30, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.02@55c9701). Click here to learn what that means.
The diff coverage is n/a.

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

@@               Coverage Diff               @@
##             branch-22.02    #9571   +/-   ##
===============================================
  Coverage                ?   10.60%           
===============================================
  Files                   ?      118           
  Lines                   ?    20082           
  Branches                ?        0           
===============================================
  Hits                    ?     2130           
  Misses                  ?    17952           
  Partials                ?        0           

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 55c9701...8f5a8d8. Read the comment docs.

@galipremsagar galipremsagar marked this pull request as ready for review November 1, 2021 04:44
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I think this looks fine. I have a few small requests that I'll trust you to address before finalizing. There is a ton of boilerplate here, but every solution that I could come up with has some sort of flaw and it's not worth postponing this PR over those kinds of improvements. We can always make them in a follow-up PR. Nice work!

cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved
cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved
cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved
cpp/tests/datetime/datetime_ops_test.cpp Outdated Show resolved Hide resolved
@bdice bdice removed their request for review November 16, 2021 20:50
@mayankanand007 mayankanand007 requested review from bdice and removed request for mayankanand007 November 16, 2021 20:50
@bdice bdice removed their request for review November 16, 2021 20:52
@skirui-source
Copy link
Contributor Author

rerun tests

@ttnghia
Copy link
Contributor

ttnghia commented Nov 17, 2021

Java test failed due to a bug in Java code. You should be good to merge this without waiting for it to be fixed.

@skirui-source
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit e08ae9c into rapidsai:branch-22.02 Nov 17, 2021
rapids-bot bot pushed a commit that referenced this pull request Nov 18, 2021
Follow-up to #9571  where we add `ceil` and `floor` support for `Series`.

Here we add `ceil` and `floor` support to `DatetimeIndex` class. This PR is dependent on #9571 getting merged first since it assumes the `libcudf` implementation for `floor` exists.

Authors:
  - Mayank Anand (https://github.com/mayankanand007)

Approvers:
  - Michael Wang (https://github.com/isVoid)
  - Ashwin Srinath (https://github.com/shwina)

URL: #9554
rapids-bot bot pushed a commit that referenced this pull request Nov 19, 2021
Small fix to inconsistent variable names in tests, following up from #9571.

Previous conversation: #9571 (comment)

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)

URL: #9735
@skirui-source skirui-source deleted the series.dt.floor branch March 12, 2022 03:31
rapids-bot bot pushed a commit that referenced this pull request Nov 1, 2022
This PR removes all "smart quotes" from the library by enforcing a pre-commit hook.

Smart quotes typically arise from copying rendered docstrings from Pandas, because Sphinx automatically transforms straight quotes into smart quotes when rendering the docs as HTML. However, the use of smart quotes is undesirable in code, and makes it difficult to do find-replace transformations if straight and smart quotes are mixed.

I have made suggestions to fix this several times before, so I am making the suggestions more permanent and automatically enforceable via a pre-commit style check:
- #12025 (comment)
- #9817 (comment)
- #9571 (comment)

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #12035
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA]Implement Series.datetime.floor
10 participants