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

[FEA] Remove FutureWarnings from Python tests #10363

Closed
17 tasks done
Tracked by #9999
bdice opened this issue Feb 25, 2022 · 3 comments · Fixed by #12406
Closed
17 tasks done
Tracked by #9999

[FEA] Remove FutureWarnings from Python tests #10363

bdice opened this issue Feb 25, 2022 · 3 comments · Fixed by #12406
Assignees
Labels
feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API. tests Unit testing for project

Comments

@bdice
Copy link
Contributor

bdice commented Feb 25, 2022

This is a meta-issue as part of #9999. Here, I catalog the current list of test files that raise a FutureWarning.

The use of a FutureWarning indicates a deprecated API, mostly from cudf or pandas. We should explicitly catch or filter out these warnings, depending on the circumstances. Eventually we will be able to run pytest -W error::FutureWarning to catch any deprecated APIs being called by our tests, which may conceal a regression in the user API.

@bdice bdice added feature request New feature or request tests Unit testing for project code quality Python Affects Python cuDF API. non-breaking Non-breaking change labels Feb 25, 2022
@bdice bdice changed the title [FEA] Remove warnings from Python tests [FEA] Remove FutureWarnings from Python tests Feb 25, 2022
@bdice bdice self-assigned this Feb 25, 2022
rapids-bot bot pushed a commit that referenced this issue Mar 9, 2022
…unaops. (#10402)

Fixes or catches a few `FutureWarning`s in Python test files. Part of #10363. (I am working through one test file at a time so we can enable `-Werr` in the future.)

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

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

URL: #10402
rapids-bot bot pushed a commit that referenced this issue Mar 9, 2022
Fixes or catches warnings in `test_rolling.py`. Part of #10363. (I am working through one test file at a time so we can enable `-Werr` in the future.)

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

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

URL: #10405
rapids-bot bot pushed a commit that referenced this issue Mar 11, 2022
Resolves part of #10363, there still are some warnings remaining, which I tried to resolve and went down a rabbit hole of a bug inside pyarrow<->pandas conversions so will take it up later.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Bradley Dice (https://github.com/bdice)

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

URL: #10416
rapids-bot bot pushed a commit that referenced this issue Mar 12, 2022
Partially addresses #10363, by removing all warnings from `test_timedelta.py`.

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

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

URL: #10418
@bdice
Copy link
Contributor Author

bdice commented Mar 12, 2022

I traced down some unexpected behavior and it turns out to be an error in the Python docs. This comment explains what I found, for posterity/reference. cc: @galipremsagar

I tried to specify a warning filter pytest -W error::FutureWarning:cudf.* on the command line. This follows the guidelines for how to specify warning filters as action:message:category:module:line.

I expected that this would raise an error for any FutureWarning raised by the cudf module or its subpackages, by interpreting the cudf.* module string as a regular expression. However, we can't do this with -W, because the input strings for message and module are escaped and not interpreted as regular expressions when defined with -W or PYTHONWARNINGS. The docs indicate that the module string is always treated as a regular expression, but that is only true for warnings.filterwarnings and not the command line option -W or environment variable PYTHONWARNINGS. This problem in the docs was reported in https://bugs.python.org/issue42272 and fixed in python/cpython#23172.

The workaround for this is to define this in our pytest configuration, setup.cfg, with the following:

[tool:pytest]
filterwarnings = error::FutureWarning:cudf.*

References:

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@github-actions
Copy link

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

rapids-bot bot pushed a commit that referenced this issue Dec 5, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12293
rapids-bot bot pushed a commit that referenced this issue Dec 5, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12305
rapids-bot bot pushed a commit that referenced this issue Dec 6, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - https://github.com/brandon-b-miller
  - Matthew Roeschke (https://github.com/mroeschke)

URL: #12304
rapids-bot bot pushed a commit that referenced this issue Dec 6, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12310
rapids-bot bot pushed a commit that referenced this issue Dec 7, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12326
rapids-bot bot pushed a commit that referenced this issue Dec 9, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12313
rapids-bot bot pushed a commit that referenced this issue Dec 10, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12324
rapids-bot bot pushed a commit that referenced this issue Dec 10, 2022
One note with these deprecations. pandas has a special value used for parameters that are defaulted, and as a result explicitly passing None will throw a warning. We don't do this, hence the difference in the warnings contexts I use for our calls vs pandas (`pytest.warns` vs `expect_warning_if`). I didn't feel like this was worth commenting on in every place, but I can if reviewers want.

Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12334
rapids-bot bot pushed a commit that referenced this issue Dec 13, 2022
I realized that my previous warning reduction PRs were causing some circular work where I would add a new warning to cudf to match pandas, which would cause those new warnings to appear in modules that I had previously declared free of warnings. To prevent this, I've changed my approach to instead go through the test modules in alphabetical order and ensure that they are all error free up to that point. This PR removes warnings from all test modules up to test_dataframe.py.

Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

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

URL: #12355
rapids-bot bot pushed a commit that referenced this issue Dec 15, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)

URL: #12381
rapids-bot bot pushed a commit that referenced this issue Dec 19, 2022
Contributes to #9999 and #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Ashwin Srinath (https://github.com/shwina)

URL: #12369
rapids-bot bot pushed a commit that referenced this issue Dec 21, 2022
Contributes to #9999 and #10363.

When I merge these changes with #12369 I no longer see any warnings on my machine. I suspect that there will be slightly different results on different machines, so we'll see have to see how CI looks after both PRs are merged before we close #10363.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Bradley Dice (https://github.com/bdice)

URL: #12406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API. tests Unit testing for project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant