[MRG+1] Fix LogisticRegression see also should include LogisticRegressiononCV#10022
[MRG+1] Fix LogisticRegression see also should include LogisticRegressiononCV#10022TomDLT merged 1 commit intoscikit-learn:masterfrom
Conversation
…onCV(scikit-learn#9995) - Added reference to LogisticRegressionCV in LogisticRegression. - Added reference to OrthogonalMatchingPursuitCV in OrthogonalMatchingPursuit. - Added references for ElasticNetCV, MultiTaskElasticNet, MultiTaskLasso. - Cross-referenced RFE and RFECV in each others docstring. - Cross-referenced CalibratedClassifierCV in _CalibrationClassifier - Added reference to LassoLarsIC in docstring of LassoLars. - Added description to references in See also section of Ridge, RidgeClassifier.
|
Thanks a lot for your PR! LGTM |
TomDLT
left a comment
There was a problem hiding this comment.
Thanks for this work, it's much appreciated !
we can add short comment/description everywhere for consistency.
It could be great. Would you like to do it?
| RidgeClassifier, RidgeCV, :class:`sklearn.kernel_ridge.KernelRidge` | ||
| RidgeClassifier : Ridge classifier | ||
| RidgeCV : Ridge regression with built-in cross validation | ||
| :class:`sklearn.kernel_ridge.KernelRidge` : Kernel ridge regression |
There was a problem hiding this comment.
Why do you use such syntax here?
What about just kernel_ridge.KernelRidge?
There was a problem hiding this comment.
Note it was already like that before and the link works properly (I checked at some point I think). I am not sure but maybe this is to ensure there is a proper link in the doc. Wild guess: maybe the link is found automatically if the name is in the same module, but maybe not if it's another subpackage.
Personally I am in favour for merging small focussed PRs like this one before too much stuff is added to the PR and ultimately they become a pain to review. Also ultimately I don't think adding description for each name has so much added value, in the html it's a link so you can click on it. If you are looking at docstring you are probably knowledgeable enough to find the source where the symbol is defined. |
|
Fair enough, merging |
|
Thanks @srajanpaliwal ! |
…cs/donigian-update-contribution-guidelines * 'master' of github.com:scikit-learn/scikit-learn: (23 commits) fixes scikit-learn#10031: fix attribute name and shape in documentation (scikit-learn#10033) [MRG+1] add changelog entry for fixed and merged PR scikit-learn#10005 issue scikit-learn#9633 (scikit-learn#10025) [MRG] Fix LogisticRegression see also should include LogisticRegressionCV(scikit-learn#9995) (scikit-learn#10022) [MRG + 1] Labels of clustering should start at 0 or -1 if noise (scikit-learn#10015) MAINT Remove redundancy in scikit-learn#9552 (scikit-learn#9573) [MRG+1] correct comparison in GaussianNB for 'priors' (scikit-learn#10005) [MRG + 1] ENH add check_inverse in FunctionTransformer (scikit-learn#9399) [MRG] FIX bug in nested set_params usage (scikit-learn#9999) [MRG+1] Fix LOF and Isolation benchmarks (scikit-learn#9798) [MRG + 1] Fix negative inputs checking in mean_squared_log_error (scikit-learn#9968) DOC Fix typo (scikit-learn#9996) DOC Fix typo: x axis -> y axis (scikit-learn#9985) improve example plot_forest_iris.py (scikit-learn#9989) [MRG+1] Deprecate pooling_func unused parameter in AgglomerativeClustering (scikit-learn#9875) DOC update news DOC Fix three typos in manifold documentation (scikit-learn#9990) DOC add missing dot in docstring DOC Add what's new for 0.19.1 (scikit-learn#9983) Improve readability of outlier detection example. (scikit-learn#9973) DOC: Fixed typo (scikit-learn#9977) ...
Reference Issues/PRs
Fixes #9995
What does this implement/fix? Explain your changes.
Added missing cross-references to CV estimators.
Any other comments?
While correcting this issue. I noticed that a lot of "See also" sections are missing short description about the references( some have descriptions and some don't). we can add short comment/description everywhere for consistency.