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

Fix warnings from pandas in test_array_ufunc.py. #10324

Merged
merged 2 commits into from
Feb 17, 2022

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Feb 17, 2022

This PR hides pandas warnings in test_array_ufunc.py. (I am working through one test file at a time so we can enable -Werr in the future.) These warnings come from specific functions like arccos for which the provided integer inputs are out-of-range. It is fine for us to hide these warnings because they do not come from cudf.

Alternatives and why I chose against them:

  • Why not limit the test data input to be in the domain of the ufunc? Limiting data inputs would hide the warning. However, it would decrease our coverage that ensures cudf and pandas agree on those results for values outside the domain of the ufunc (particularly trigonometric functions). It would additionally require inputs to be generated based on the ufunc, which would be lengthy.
  • Why not make cudf raise a warning and ensure that pandas/cudf agree on warning behavior? I chose against this because we generally avoid data introspection in cudf, and this would involve a significantly more complex approach to ufunc execution.

@bdice bdice requested a review from a team as a code owner February 17, 2022 20:23
@github-actions github-actions bot added the Python Affects Python cuDF API. label Feb 17, 2022
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 17, 2022
@bdice bdice self-assigned this Feb 17, 2022
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.

Lol when I merged these changes I definitely wondered whether you would find these as part of your warnings hunt before I could get around to fixing them once I finalize ufuncs. Thanks for the fix.

Is there a message regex that we could use in filterwarnings to ensure that we only catch the expected warnings (out of range args for inverse sin functions, etc)? Not a must, just a suggestion if it's easy. I don't remember exactly what warnings come up but I believe that all the functions that you have put into the list are domain errors.

@bdice
Copy link
Contributor Author

bdice commented Feb 17, 2022

@vyasr In bb18841, I catch specific warning messages.

I just started at this file because it was alphabetically first -- not trying to interfere with your ufunc work! 😋

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #10324 (bb18841) into branch-22.04 (a7d88cd) will increase coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04   #10324      +/-   ##
================================================
+ Coverage         10.42%   10.63%   +0.21%     
================================================
  Files               119      122       +3     
  Lines             20603    20940     +337     
================================================
+ Hits               2148     2228      +80     
- Misses            18455    18712     +257     
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 56 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 fdad597...bb18841. Read the comment docs.

@bdice
Copy link
Contributor Author

bdice commented Feb 17, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b28bad6 into rapidsai:branch-22.04 Feb 17, 2022
@bdice bdice changed the title Hide warnings from pandas in test_array_ufunc.py. Fix warnings from pandas in test_array_ufunc.py. Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants