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 tests of reflected ufuncs and fix behavior of logical reflected ufuncs #10261

Merged
merged 5 commits into from
Feb 11, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Feb 10, 2022

This PR fixes cases like cupy.array([1, 2, 3]) < cudf.Series([1, 2, 3]). The code would previously attempt to find a reflected operator Series.__rle__, but no such operator exists because of the inherent symmetry in comparison operators. These changes fix the detection of such operators to just use operator __ge__ in this case.

@vyasr vyasr added bug Something isn't working 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. non-breaking Non-breaking change labels Feb 10, 2022
@vyasr vyasr self-assigned this Feb 10, 2022
@vyasr vyasr requested a review from a team as a code owner February 10, 2022 00:07
@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #10261 (6ee5487) into branch-22.04 (a7d88cd) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 6ee5487 differs from pull request most recent head f516dfa. Consider uploading reports for the commit f516dfa to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##           branch-22.04   #10261   +/-   ##
=============================================
  Coverage         10.42%   10.42%           
=============================================
  Files               119      122    +3     
  Lines             20603    20588   -15     
=============================================
- Hits               2148     2147    -1     
+ Misses            18455    18441   -14     
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/fuzzer.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/main.py 0.00% <ø> (ø)
python/cudf/cudf/_version.py 0.00% <ø> (ø)
python/cudf/cudf/comm/gpuarrow.py 0.00% <ø> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/column.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/methods.py 0.00% <ø> (ø)
... and 58 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 19dc46f...f516dfa. Read the comment docs.

python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving because this fixes a bug that is blocking Dask-SQL. A couple small comments below.

python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Show resolved Hide resolved
@vyasr vyasr force-pushed the feature/more_operators branch from bc2adad to f516dfa Compare February 11, 2022 01:23
@galipremsagar
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 4c92f86 into rapidsai:branch-22.04 Feb 11, 2022
@vyasr vyasr deleted the feature/more_operators branch March 9, 2022 17:22
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 bug Something isn't working non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants