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

Convert deprecation warnings to errors on CI #26

Open
jakirkham opened this issue Mar 6, 2024 · 7 comments
Open

Convert deprecation warnings to errors on CI #26

jakirkham opened this issue Mar 6, 2024 · 7 comments

Comments

@jakirkham
Copy link
Member

To make it easier to catch and fix deprecations in RAPIDS projects, it is worth considering converting deprecation warnings to errors on CI. That way deprecations fail loudly and we are able to catch and address them quickly. Alternatively we can use that opportunity to tighten our dependencies and flag the deprecation for follow up when we are ready.

rapids-bot bot pushed a commit to rapidsai/cuml that referenced this issue Mar 11, 2024
The test suite will now fail when a warning comes from cudf as apart of rapidsai/build-planning#26.

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #5796
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue Mar 12, 2024
… are casted to the object's type) (#1358)

In 24.04, cudf issues a `FutureWarning` when setting a value to a column would change the data type of the original column. The fix involves casting the original column to the value's type first or vice versa. Could use a second eye on the correct direction to cast if a data type was used intentionally.

Additionally the test suite will now fail when a warning comes from cudf as apart of rapidsai/build-planning#26

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

Approvers:
  - Mark Harris (https://github.com/harrism)
  - H. Thomson Comer (https://github.com/thomcom)

URL: #1358
rapids-bot bot pushed a commit to rapidsai/cugraph that referenced this issue Mar 17, 2024
In 24.04, cudf issues a FutureWarning when using positional indexing with `__getitem__`. The change here is to use `iloc` instead.

Additionally the test suite will now fail when a warning comes from cudf as apart of rapidsai/build-planning#26

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Ralph Liu (https://github.com/nv-rliu)
  - Brad Rees (https://github.com/BradReesWork)

Approvers:
  - Rick Ratzel (https://github.com/rlratzel)
  - Don Acosta (https://github.com/acostadon)

URL: #4223
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue Apr 16, 2024
…ted `geopandas.dataset` module) (#1360)

Continuing the effort in rapidsai/build-planning#26 by enabling `FutureWarnings` and `DeprecationWarnings` to raise errors in the CI.

The primary change here is to replace `geopandas.dataset` usage with the files used (the data in https://github.com/geopandas/geopandas/tree/main/geopandas/tests/data)

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

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Jake Awe (https://github.com/AyodeAwe)
  - Bradley Dice (https://github.com/bdice)

URL: #1360
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this issue May 1, 2024
The test suite will now fail on FutureWarnings and DeprecationWarnings as apart of rapidsai/build-planning#26.

pytest.ini was consolidated into pyproject.toml so there's only 1 place with these configs.

There are some `TODO`s including scikit-learn warnings, but I hope it's OK to potentially pass the baton to @betatim :)

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #5877
rapids-bot bot pushed a commit to rapidsai/rmm that referenced this issue May 2, 2024
As part of rapidsai/build-planning#26, warnings in Python tests will cause that test to fail.

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Lawrence Mitchell (https://github.com/wence-)

Approvers:
  - Lawrence Mitchell (https://github.com/wence-)

URL: #1551
rapids-bot bot pushed a commit to rapidsai/kvikio that referenced this issue May 2, 2024
As part of rapidsai/build-planning#26, warnings from the Python test suite will cause a test to fail

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

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - Lawrence Mitchell (https://github.com/wence-)

URL: #375
rapids-bot bot pushed a commit to rapidsai/cuvs that referenced this issue May 2, 2024
Part of rapidsai/build-planning#26, warnings in Python tests will now be treated as errors

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #102
@jameslamb
Copy link
Member

This is awesome! I'm also happy to see the consolidation into pyproject.toml and eliminating pytest.ini.

I see one that might have been missed... what about raft?

rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue May 3, 2024
Following rapidsai/build-planning#26, enabled warnings-as-errors for cuproj test

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

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

URL: #1379
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this issue May 3, 2024
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this issue May 4, 2024
rapids-bot bot pushed a commit to rapidsai/cuxfilter that referenced this issue May 6, 2024
Part of rapidsai/build-planning#26

Luckily it doesn't appear the current test suite has any :)

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

Approvers:
  - Ajay Thorve (https://github.com/AjayThorve)

URL: #595
rapids-bot bot pushed a commit to rapidsai/raft that referenced this issue May 8, 2024
As part of rapidsai/build-planning#26, warnings in Python tests will be converted to test failures


`ignore:Unknown pytest.mark.ucx:PytestUnknownMarkWarning` could be removed once #2281 is merged cc @jameslamb

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #2288
abc99lr pushed a commit to abc99lr/raft that referenced this issue May 10, 2024
As part of rapidsai/build-planning#26, warnings in Python tests will be converted to test failures


`ignore:Unknown pytest.mark.ucx:PytestUnknownMarkWarning` could be removed once rapidsai#2281 is merged cc @jameslamb

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

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#2288
rapids-bot bot pushed a commit to rapidsai/cucim that referenced this issue May 14, 2024
As part of rapidsai/build-planning#26, enabling Python test failures for `FutureWarning`s and `DeprecationWarning`s

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Gigon Bae (https://github.com/gigony)

Approvers:
  - https://github.com/jakirkham

URL: #734
@mroeschke
Copy link

Did I miss any RAPIDS repos not covered in #26 (comment)?

Additionally, to ensure that future RAPIDS repos have this change, is there like a template repo where we could enable this by default?

@jakirkham
Copy link
Member Author

Thanks Matt for handling this for Python! 🙏

Is there anything we still need to do on the C++ side?

@mroeschke
Copy link

Is there anything we still need to do on the C++ side?

Yeah I think all RAPIDS libraries should have -Wdeprecated-declarations enabled by default e.g. rapidsai/cudf#14819.

@vyasr
Copy link
Contributor

vyasr commented Jul 22, 2024

Not all RAPIDS libraries have this on by default yet. In particular, I think we'll have to update cuml/cugraph. This may still require some real C++ work to update the API calls though. I don't know if you want to track that effort in this issue, close this out and open repo-specific ones for the remaining cases, or some combination of the two. Most likely you'll want help from C++ library devs to address the outstanding issues there, though.

@jakirkham
Copy link
Member Author

Think it is find to use this as an umbrella issue for the work needed to reach this goal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants