[MRG] Created 'cross-validation estimator' entry in glossary #11661
Conversation
sklearn/calibration.py
Outdated
| @@ -28,7 +28,8 @@ | |||
|
|
|||
|
|
|||
| class CalibratedClassifierCV(BaseEstimator, ClassifierMixin): | |||
| """Probability calibration with isotonic regression or sigmoid. | |||
| """:term:`Cross-validated <cross-validation estimator>` Probability | |||
| @@ -1420,12 +1418,13 @@ class ElasticNetCV(LinearModelCV, RegressorMixin): | |||
| Length of the path. ``eps=1e-3`` means that | |||
| ``alpha_min / alpha_max = 1e-3``. | |||
|
|
|||
| n_alphas : int, optional | |||
| n_alphas : int, optional (default=10) | |||
There was a problem hiding this comment.
It seems kinda odd that here some parameters mention their default in the docstring, some don't.
There was a problem hiding this comment.
Yes thanks, it's also unrelated to this PR so I reverted the changes.
There was a problem hiding this comment.
All parameters should mention defaults at all times. But it's unrelated, so I don't mind.
|
I like the entry, not sure how I feel about violating some pep and having a line break in the first line of the docstring. No strong feelings, though. |
|
@jnothman thoughts? |
sklearn/covariance/graph_lasso_.py
Outdated
| @@ -467,7 +467,8 @@ def graphical_lasso_path(X, alphas, cov_init=None, X_test=None, mode='cd', | |||
|
|
|||
|
|
|||
| class GraphicalLassoCV(GraphicalLasso): | |||
| """Sparse inverse covariance w/ cross-validated choice of the l1 penalty | |||
| """Sparse inverse covariance with :term:`cross-validated choice | |||
There was a problem hiding this comment.
I don't think these references should be in the first line of a docstring. First lines have special roles in pydoc and elsewhere. They should be just one line, according to PEP257.
There was a problem hiding this comment.
Thanks for the feedback, would that be OK if I revert the first docstring line and add a new one saying something like:
See glossary entry for cross-validation estimators.
(with link included)
?
doc/glossary.rst
Outdated
| @@ -955,6 +955,20 @@ such as: | |||
| and do not provide :term:`set_params` or :term:`get_params`. | |||
| Parameter validation may be performed in ``__init__``. | |||
|
|
|||
| cross-validation estimator | |||
| An estimator that has built-in cross-validation capabilities to | |||
| automatically select the best hyper-parameters, e.g. | |||
doc/glossary.rst
Outdated
| automatically select the best hyper-parameters, e.g. | ||
| :class:`ElasticNetCV <linear_model.ElasticNetCV>`, or | ||
| :class:`LogisticRegressionCV <linear_model.LogisticRegressionCV>`. | ||
| Cross-validation estimators are named *EstimatorCV*. The advantage of |
There was a problem hiding this comment.
It might be helpful to make this more concrete, with a statement like:
That is, ``EstimatorCV()`` tends to be roughly equivalent to
``GridSearchCV(Estimator(), {'parm': [0, 1, 2, 3, ...]})``
Then again, maybe that adds confusion.
…learn#11661) * Created 'cross-validation estimator' in glossary and referenced it where needed * Addressed adrinjalali comments * updated glossary entry * updaed docstrings according to commets
Reference Issues/PRs
Closes #11659
What does this implement/fix? Explain your changes.
This PR creates a 'cross-validation estimator' entry in glossary and adds references in every
EstimatorCVclass.Any other comments?
Not everything from #11659 is addressed here, but I believe it makes more sense to close it with this PR and address the other points in other PRs.