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

Add quarters to libcudf datetime #8779

Merged
merged 11 commits into from
Jul 26, 2021

Conversation

shaneding
Copy link
Contributor

@shaneding shaneding commented Jul 19, 2021

Partly addresses #8676.
This PR adds date_time:: extract_quarter. The function returns an int (1,2,3,4) depending on which quarter of the year the date is in. Also fixed an incorrect date in previous testcase (#8711).

@shaneding shaneding requested a review from a team as a code owner July 19, 2021 22:40
@shaneding shaneding requested review from robertmaynard and codereport and removed request for a team July 19, 2021 22:40
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Jul 19, 2021
@shaneding shaneding added feature request New feature or request non-breaking Non-breaking change labels Jul 19, 2021
@ttnghia
Copy link
Contributor

ttnghia commented Jul 20, 2021

As discussed many times in the past, please add more description clarifying what this PR is doing. Otherwise, the reviewers may have no idea about what to review. Just a link to an issue is not enough.

cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved
cpp/src/datetime/datetime_ops.cu Outdated Show resolved Hide resolved
@shaneding shaneding changed the title Added quarters to libcudf datetime Add quarters to libcudf datetime Jul 21, 2021
cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved
cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved
cpp/tests/datetime/datetime_ops_test.cpp Outdated Show resolved Hide resolved
cpp/tests/datetime/datetime_ops_test.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ttnghia ttnghia left a comment

Choose a reason for hiding this comment

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

Sorry, (maybe) just one more time 😃

cpp/include/cudf/datetime.hpp Outdated Show resolved Hide resolved
cpp/tests/datetime/datetime_ops_test.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

Just one small comment.

cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved
cpp/src/datetime/datetime_ops.cu Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 22, 2021

Codecov Report

Merging #8779 (1add7de) into branch-21.10 (18f7c01) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

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

@@               Coverage Diff                @@
##           branch-21.10    #8779      +/-   ##
================================================
- Coverage         10.67%   10.59%   -0.09%     
================================================
  Files               110      116       +6     
  Lines             18271    19032     +761     
================================================
+ Hits               1951     2017      +66     
- Misses            16320    17015     +695     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/lists.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/numerical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/struct.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/dataframe.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
... and 74 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 74d2517...c2bbce6. Read the comment docs.

@shaneding shaneding added the 3 - Ready for Review Ready for review by team label Jul 23, 2021
@shaneding shaneding requested a review from codereport July 23, 2021 14:37
@shaneding shaneding self-assigned this Jul 23, 2021
@shaneding shaneding added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jul 25, 2021
@shaneding
Copy link
Contributor Author

rerun tests

@shaneding
Copy link
Contributor Author

@gpucibot merge

1 similar comment
@beckernick
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 756499f into rapidsai:branch-21.10 Jul 26, 2021
rapids-bot bot pushed a commit that referenced this pull request Aug 10, 2021
Closes #8676.
This PR adds python bindings for #8779.

Authors:
  - https://github.com/shaneding

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

URL: #8862
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants