Skip to content
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

DOC improve model evaluation a bit #18166

Merged
merged 5 commits into from Aug 17, 2020

Conversation

lorentzenchr
Copy link
Member

What does this implement/fix? Explain your changes.

This improves the User Guide about model evaluation a little bit.

Any other comments?

Inspired by reading #18051.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lorentzenchr minors nits but LGTM!

@@ -117,7 +117,7 @@ Usage examples:

.. note::

The values listed by the ValueError exception correspond to the functions measuring
The values listed by the ``ValueError`` exception correspond to the functions measuring
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI we changed the meaning of single backquotes so that they have the same effect of double backquotes. I tend to use them because it's easier to read the plain text, but we don't have specific guidelines for this so do as you prefer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do single and double backquotes have different semantics?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are exactly the same now.

predictions.
This function returns the mean squared error of the actual outcome
:math:`y \in \{0,1\}` and the predicted probability estimate
:math:`p = \operatorname{Pr}(y = 1)` (``predict_proba``) as score:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We like to link to the glossary when it might make sense

Suggested change
:math:`p = \operatorname{Pr}(y = 1)` (``predict_proba``) as score:
:math:`p = \operatorname{Pr}(y = 1)` (:term:`predict_proba`) as score:

I would also add "as outputed by ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced "as score" by "as outputted by" because, as you pointed out, it is a loss, not a score in scikit-learn terms.

doc/modules/model_evaluation.rst Outdated Show resolved Hide resolved
doc/modules/model_evaluation.rst Outdated Show resolved Hide resolved
doc/modules/model_evaluation.rst Outdated Show resolved Hide resolved
doc/modules/model_evaluation.rst Outdated Show resolved Hide resolved
@lorentzenchr
Copy link
Member Author

@NicolasHug Thanks for your fast review.

@@ -117,7 +117,7 @@ Usage examples:

.. note::

The values listed by the ValueError exception correspond to the functions measuring
The values listed by the ``ValueError`` exception correspond to the functions measuring
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are exactly the same now.

doc/modules/model_evaluation.rst Show resolved Hide resolved
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM

doc/modules/model_evaluation.rst Outdated Show resolved Hide resolved
@lorentzenchr
Copy link
Member Author

@thomasjpfan Thanks for review an approval.
Should be ready for merge.

@NicolasHug NicolasHug merged commit e0abd26 into scikit-learn:master Aug 17, 2020
@lorentzenchr lorentzenchr deleted the doc_evaluation branch August 17, 2020 17:51
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants