Remove the deprecated convert_dtype kwarg - #8436
Merged
Merged
Conversation
This removes the deprecated `convert_dtype` kwarg everywhere.
jcrist
added
improvement
4 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesconvert_dtype API removal
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/cuml/cuml/solvers/sgd.pyx (1)
99-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the stale
convert_to_dtypeentry from thefit_sgddocstring.The
fit_sgdsignature no longer accepts this keyword, but the docstring still documents it at Line [129]. A caller that follows the docstring gets an unexpected-keyword error. Delete the two-line entry.Proposed fix
- convert_to_dtype : bool, default=True - When set to True, will convert array inputs to be of the proper dtypes.Based on learnings,
fit_sgdis an internal solver helper, so this finding is a documentation-contract defect rather than a required public deprecation period.Also applies to: 129-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/cuml/solvers/sgd.pyx` around lines 99 - 104, Remove the stale two-line convert_to_dtype parameter entry from the fit_sgd docstring, leaving the documentation consistent with the current fit_sgd signature and all remaining parameter descriptions unchanged.Source: Learnings
🧹 Nitpick comments (1)
python/cuml/cuml/dask/neighbors/kneighbors_regressor.py (1)
99-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the helper parameter names with the worker API.
The task submission supplies
self.n_outputsat Line [195] and the worker rank at Line [196]. The worker method usesn_outputsat Line [57] andrankat Line [58] inpython/cuml/cuml/neighbors/kneighbors_regressor_mg.pyx.The current values arrive correctly only because
_func_predictnames the arguments in reverse. Rename the helper parameters ton_outputs, rank. This prevents a future positional-argument regression.Suggested parameter-order cleanup
def _func_predict( model, index, index_parts_to_ranks, index_nrows, query, query_parts_to_ranks, query_nrows, ncols, - rank, - n_output, + n_outputs, + rank, ): return model.predict( index, index_parts_to_ranks, index_nrows, query, query_parts_to_ranks, query_nrows, ncols, - rank, - n_output, + n_outputs, + rank, )Also applies to: 194-197
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/cuml/dask/neighbors/kneighbors_regressor.py` around lines 99 - 110, Rename the corresponding parameters of the _func_predict helper to n_outputs and rank, matching the worker API and the positional arguments supplied by the task submission. Update the helper’s model.predict invocation to pass these renamed parameters in the existing API order, without changing other behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@python/cuml/cuml/solvers/sgd.pyx`:
- Around line 99-104: Remove the stale two-line convert_to_dtype parameter entry
from the fit_sgd docstring, leaving the documentation consistent with the
current fit_sgd signature and all remaining parameter descriptions unchanged.
---
Nitpick comments:
In `@python/cuml/cuml/dask/neighbors/kneighbors_regressor.py`:
- Around line 99-110: Rename the corresponding parameters of the _func_predict
helper to n_outputs and rank, matching the worker API and the positional
arguments supplied by the task submission. Update the helper’s model.predict
invocation to pass these renamed parameters in the existing API order, without
changing other behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a2fd106-22af-4253-b95c-d22a6fb73ad0
📒 Files selected for processing (62)
python/cuml/cuml/cluster/agglomerative.pyxpython/cuml/cuml/cluster/dbscan.pyxpython/cuml/cuml/cluster/hdbscan/hdbscan.pyxpython/cuml/cuml/cluster/kmeans.pyxpython/cuml/cuml/common/doc_utils.pypython/cuml/cuml/covariance/empirical_covariance.pypython/cuml/cuml/covariance/ledoit_wolf.pypython/cuml/cuml/dask/ensemble/base.pypython/cuml/cuml/dask/ensemble/randomforestclassifier.pypython/cuml/cuml/dask/ensemble/randomforestregressor.pypython/cuml/cuml/dask/manifold/umap.pypython/cuml/cuml/dask/neighbors/kneighbors_classifier.pypython/cuml/cuml/dask/neighbors/kneighbors_regressor.pypython/cuml/cuml/dask/neighbors/nearest_neighbors.pypython/cuml/cuml/decomposition/incremental_pca.pypython/cuml/cuml/decomposition/pca.pyxpython/cuml/cuml/decomposition/tsvd.pyxpython/cuml/cuml/ensemble/randomforestclassifier.pypython/cuml/cuml/ensemble/randomforestregressor.pypython/cuml/cuml/explainer/tree_shap.pyxpython/cuml/cuml/internals/validation.pypython/cuml/cuml/kernel_ridge/kernel_ridge.pypython/cuml/cuml/linear_model/base.pypython/cuml/cuml/linear_model/elastic_net.pypython/cuml/cuml/linear_model/lars.pyxpython/cuml/cuml/linear_model/linear_regression.pyxpython/cuml/cuml/linear_model/logistic_regression.pypython/cuml/cuml/linear_model/mbsgd_classifier.pypython/cuml/cuml/linear_model/mbsgd_regressor.pypython/cuml/cuml/linear_model/ridge.pyxpython/cuml/cuml/manifold/t_sne.pyxpython/cuml/cuml/manifold/umap/umap.pyxpython/cuml/cuml/metrics/cluster/adjusted_rand_index.pyxpython/cuml/cuml/metrics/cluster/silhouette_score.pyxpython/cuml/cuml/metrics/confusion_matrix.pypython/cuml/cuml/metrics/kl_divergence.pyxpython/cuml/cuml/metrics/pairwise_distances.pyxpython/cuml/cuml/metrics/pairwise_kernels.pypython/cuml/cuml/metrics/trustworthiness.pyxpython/cuml/cuml/naive_bayes/naive_bayes.pypython/cuml/cuml/neighbors/kernel_density.pyxpython/cuml/cuml/neighbors/kneighbors_classifier.pyxpython/cuml/cuml/neighbors/kneighbors_classifier_mg.pyxpython/cuml/cuml/neighbors/kneighbors_regressor.pyxpython/cuml/cuml/neighbors/kneighbors_regressor_mg.pyxpython/cuml/cuml/neighbors/nearest_neighbors.pyxpython/cuml/cuml/neighbors/nearest_neighbors_mg.pyxpython/cuml/cuml/random_projection/random_projection.pypython/cuml/cuml/solvers/cd.pyxpython/cuml/cuml/solvers/qn.pyxpython/cuml/cuml/solvers/sgd.pyxpython/cuml/cuml/svm/linear.pyxpython/cuml/cuml/svm/linear_svc.pypython/cuml/cuml/svm/linear_svr.pypython/cuml/cuml/svm/svc.pypython/cuml/cuml/svm/svm_base.pyxpython/cuml/cuml/svm/svr.pypython/cuml/cuml/tsa/arima.pyxpython/cuml/cuml/tsa/auto_arima.pyxpython/cuml/cuml/tsa/seasonality.pypython/cuml/cuml/tsa/stationarity.pyxpython/cuml/tests/test_validation.py
💤 Files with no reviewable changes (2)
- python/cuml/cuml/metrics/pairwise_kernels.py
- python/cuml/cuml/tsa/stationarity.pyx
csadorf
requested changes
Aug 3, 2026
csadorf
left a comment
csadorf
left a comment
Contributor
There was a problem hiding this comment.
Please also remove the related guidance from wiki/python/DEVELOPER_GUIDE.md and wiki/python/ESTIMATOR_GUIDE.md.
Member
Author
|
Done. |
Member
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes the deprecated
convert_dtypekwarg everywhere.Part of #8225.