Deprecate cupy array backend (#843)#881
Conversation
Emit DeprecationWarning when the cupy backend is selected, without removing functionality: - Options.set_option warns for ARRAY_BACKEND='cupy' and for 'cupy' in ARRAY_PRIORITY. - Common.set_backend warns when backend='cupy'. Adds tests asserting the warnings fire and that non-cupy backends do not warn.
priyam0k
requested review from
genedan,
henrydingliu,
jbogaardt and
kennethshsu
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 69ab9b0. Configure here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #881 +/- ##
==========================================
- Coverage 87.04% 86.99% -0.05%
==========================================
Files 86 87 +1
Lines 4986 4952 -34
Branches 646 629 -17
==========================================
- Hits 4340 4308 -32
+ Misses 456 454 -2
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Move the DeprecationWarning to set_backend's public entry point and add a private _warn guard so internal recursive (inplace) and deep-child calls do not re-warn. This makes stacklevel=2 point at the user's call site and ensures exactly one warning fires. Strengthens the test to assert a single warning at the caller's file.
|
@genedan I'd like to defer this review to you. |
|
gentle bump on this one whenever you get a chance @genedan — it follows the three hookpoints you outlined on #843 (the two set_option paths + set_backend), warning-only per @henrydingliu's note, with a small test confirming the warnings fire. ci's green. if it looks good i'm happy to do the matching dask deprecation (#842) next so they stay consistent. |
|
I've added my review notes, could you address them? |
priyam0k
changed the title
4 participants
Summary of Changes
Emits a
DeprecationWarningwhen the cupy array backend is selected, without removing any functionality. cupy is elective and not well maintained, so it's slated for removal in a future release (#601). This warns users now so they can migrate before it's gone.Three entry points warn:
set_option("ARRAY_BACKEND", "cupy")warnsset_option("ARRAY_PRIORITY", ...)warns only when cupy is ranked ahead of numpy or sparse, so valid priority lists don't get noisyTriangle.set_backend("cupy")warns once at the user's call site, even withdeep=True. Internal recursive calls are suppressed via a private_warnflag.Also documented the
_warnanddeepparams onset_backend.Added tests in
chainladder/utils/tests/test_utilities.pycovering each warning path plus the no-warn cases (non-cupy backends, and cupy ranked last in priority).Related GitHub Issue(s)
Closes #843.
Per @henrydingliu's call on the issue, this only deprecates (warns) and does not remove cupy. Removal can follow later under #601.
Additional Context for Reviewers
The current default priority
["dask", "sparse", "cupy", "numpy"]has cupy ahead of numpy, so explicitly re-setting that default would warn. Happy to switch the check to only fire when cupy leads both numpy and sparse if you'd prefer the default never warns.uv run --extra dev pytest -k "deprecat or cupy"passed, 6 testsuv run jb build docs --builder=custom --custom-builder=doctestcleanI passed tests locally for both code (
uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)