-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG+1] Fix LogisticRegression see also should include LogisticRegressiononCV #10022
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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you use such syntax here?
What about just kernel_ridge.KernelRidge
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.