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

Support more units in cudf.DateOffset #7078

Merged

Conversation

brandon-b-miller
Copy link
Contributor

@brandon-b-miller brandon-b-miller commented Jan 5, 2021

Enhancements for cudf.DateOffset

  • Adds more units for DateOffset
  • Allows specifying multiple units when constructing a DateOffset
  • Fix for reflected binaryops with DateOffsets

@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Merging #7078 (b40930e) into branch-0.20 (51336df) will decrease coverage by 0.44%.
The diff coverage is 82.22%.

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

@@               Coverage Diff               @@
##           branch-0.20    #7078      +/-   ##
===============================================
- Coverage        82.88%   82.44%   -0.45%     
===============================================
  Files              103      103              
  Lines            17668    17432     -236     
===============================================
- Hits             14645    14371     -274     
- Misses            3023     3061      +38     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/__init__.py 100.00% <ø> (ø)
python/cudf/cudf/io/orc.py 86.80% <ø> (-0.10%) ⬇️
python/cudf/cudf/utils/cudautils.py 55.04% <25.00%> (-2.72%) ⬇️
python/cudf/cudf/utils/dtypes.py 81.87% <41.66%> (-1.57%) ⬇️
python/cudf/cudf/core/column/numerical.py 94.41% <70.00%> (-0.02%) ⬇️
python/cudf/cudf/core/tools/datetimes.py 80.00% <74.07%> (-4.54%) ⬇️
python/cudf/cudf/core/column/column.py 88.48% <75.00%> (-0.17%) ⬇️
python/cudf/cudf/core/groupby/groupby.py 91.39% <76.92%> (-0.06%) ⬇️
python/cudf/cudf/core/column/decimal.py 90.08% <78.26%> (-2.85%) ⬇️
python/dask_cudf/dask_cudf/backends.py 89.20% <80.00%> (-0.38%) ⬇️
... and 61 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 b8baed5...e0f1b5c. Read the comment docs.

@shwina shwina added non-breaking Non-breaking change feature request New feature or request labels Jan 8, 2021
@shwina shwina marked this pull request as ready for review January 8, 2021 16:44
@shwina shwina requested a review from a team as a code owner January 8, 2021 16:44
@shwina shwina requested review from shwina and galipremsagar January 8, 2021 16:44
@shwina shwina added non-breaking Non-breaking change feature request New feature or request and removed non-breaking Non-breaking change feature request New feature or request labels Apr 13, 2021
@shwina
Copy link
Contributor

shwina commented Apr 14, 2021

rerun tests

2 similar comments
@shwina
Copy link
Contributor

shwina commented Apr 14, 2021

rerun tests

@kkraus14
Copy link
Collaborator

rerun tests

@vyasr
Copy link
Contributor

vyasr commented Apr 14, 2021

I believe all CI is stalled right now due to the connectivity problems we're experiencing across the board preventing environments from even installing correctly.

@shwina
Copy link
Contributor

shwina commented Apr 15, 2021

rerun tests

@vyasr
Copy link
Contributor

vyasr commented Apr 16, 2021

Minor note, if #7845 gets merged before this PR then we should address this comment before this PR gets merged. I'll take care of it in #7845 if this PR gets merged first.

@brandon-b-miller since #7845 has been merged, when you next merge branch-0.20 into this PR you'll want to address this change. Specifically, since in this PR cudf.DateOffset no longer inherits from pandas.DateOffset and cudf._DateOffsetScalars has been deleted, you can replace the object.__setattr__ call in this line with just a normal self._scalars = scalars while resolving conflicts. This is already what the code looks like here AFAICT, just letting you know in case the merge looks unexpected.

@brandon-b-miller brandon-b-miller added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Apr 22, 2021
@brandon-b-miller
Copy link
Contributor Author

this should be ready to merge :)

@brandon-b-miller
Copy link
Contributor Author

@kkraus14 @galipremsagar

@galipremsagar
Copy link
Contributor

@brandon-b-miller should we just rerun tests before we hit merge because the run seems to be 3 days old? Just being cautious here.

@brandon-b-miller
Copy link
Contributor Author

rerun tests

@galipremsagar
Copy link
Contributor

@brandon-b-miller should we just rerun tests before we hit merge because the run seems to be 3 days old? Just being cautious here.

🤦 It got triggered because of the phrase I used

@galipremsagar
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 50368de into rapidsai:branch-0.20 Apr 22, 2021
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 non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants