ENH Add CalibrationDisplay plotting class by lucyleeow · Pull Request #17443 · scikit-learn/scikit-learn

lucyleeow · 2020-06-04T11:31:34Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Adds CalibrationDisplay for binary classifiers with visualization API

Any other comments?

Plot currently looks like this:

Yet to add tests and update examples as I am unsure about API:

Should a histogram be added? If so should the histogram be a separate plot (e.g., here) or on the same plot (as suggested by @amueller: #8425 (comment)).
If we put it on the same plot I'm worried it will be too crowded. If we want 2 separate plots, the API becomes difficult as we should have 2 different plot **kwargs parameters, for both plots so people can amend them separately. You also couldn't use ax = plt.gca() to get the current axis when you want to add lines to an existing plot (like for the current plots, e.g., here). I think you could use CalibrationDisplay.ax_ though.

NicolasHug

Thanks @lucyleeow , made a quick pass but looks good

Regarding the histograms: since it's just a simple call the plt.hist, I think we should let users call that and rely on the prob_pred attribute of the Vizualiser. Since this would be illustrated in the examples, it's fine IMO.

sklearn/metrics/_plot/calibration_curve.py

glemaitre

I post these comments. I see this is still WIP.

glemaitre · 2020-06-04T14:04:04Z

sklearn/metrics/_plot/calibration_curve.py

+        self.prob_pred = prob_pred
+        self.estimator_name = estimator_name
+
+    @_deprecate_positional_args


We would need this deprecation since this is a new class and new function?

Should I move the * to not allow any positional args..? (bit confused about this)

Specifically here def plot(self, ax=None, *, name=None, ref_line=True, **kwargs): as ax arguably should/could be keyword.

sklearn/metrics/_plot/calibration_curve.py

lucyleeow · 2020-06-04T18:13:37Z

Thanks for the reviews guys! I would like to update the examples plot_calibration_curve.py and plot_compare_calibration.py to notebook style and add links, as well as amending them to use plot_calibration_curve.

Can I do the non-plot_calibration_curve changes in this PR or should I do it in a different PR?

NicolasHug · 2020-06-04T18:19:31Z

I'm fine with doing this here so we can see it in action.

Regarding the module, I'm not sure metric is the right choice here. I think we should either use sklearn.inspection, or transform calibration into a sub-package?

lucyleeow · 2020-06-04T20:19:12Z

Good point, it's not really a metric. sklearn.inspection sounds reasonable, from the user guide:

The sklearn.inspection module provides tools to help understand the predictions from a model and what affects them. This can be used to evaluate assumptions and biases of a model, design a better model, or to diagnose issues with model performance.

I'll wait a bit to see if there are any objections and if there are none, I'll move it there.

lucyleeow · 2020-06-05T08:56:57Z

@NicolasHug would it be appropriate to put this inside sklearn/calibration.py and sklearn/tests/test_calibration.py or should this plotting code be in it's own files?

lucyleeow · 2021-07-22T03:54:03Z

ping @glemaitre changes made, thanks! (and the CIs are greeeeen!)

glemaitre · 2021-07-22T06:56:33Z

LGTM apart from the small comment where I would check what failing message do we get.

glemaitre · 2021-07-22T06:57:56Z

I will not merge right now. I would like to know if @ogrisel +1 is still standing after the changes.

lucyleeow · 2021-07-28T07:22:35Z

ping @ogrisel...?

lorentzenchr · 2021-08-15T10:22:50Z

@ogrisel Can we merge?

lorentzenchr · 2021-08-23T06:55:17Z

@adrinjalali I added this PR to the 1.0 milestone as I think this should really be in. Already +2 and just waiting for a final OK of @ogrisel as there have been some changes since his approval.

lucyleeow · 2021-08-23T07:53:44Z

Yes please!

doc/whats_new/v1.0.rst

ogrisel

There is now a failing test, not sure why:

In sklearn/calibration.py at line 650:

        if y_pred.ndim != 1:  # `predict_proba`
            if y_pred.shape[1] != 2:
>               raise ValueError(classification_error)
E               ValueError: Expected 'estimator' to be a binary classifier, but got DecisionTreeClassifier

Beyond fixing the test failure, I think the error message should be improved to report the observed shape of y_pred.

sklearn/calibration.py

glemaitre · 2021-08-30T12:16:44Z

The failure is due to the fact that we already improved the previous error message in another PR.
Here, we could maybe catch this message and add the part that @ogrisel would like about y_pred.

ogrisel

Another detail to fix below.

Other than that and the failing test, I confirm that this PR looks good to me. Thanks again @lucyleeow.

examples/calibration/plot_calibration_curve.py

lucyleeow · 2021-08-31T06:20:09Z

sklearn/metrics/_plot/base.py

+                f"{classification_error} fit on multiclass ({y_pred_shape} classes)"
+                " data"


Happy to change this message if you had something different in mind @ogrisel.

Thanks, the message looks good!

lucyleeow · 2021-08-31T06:20:46Z

Thank you @ogrisel @glemaitre, changes made!

ogrisel · 2021-08-31T07:43:25Z

Merged! Thank you very much @lucyleeow!

lucyleeow · 2021-08-31T07:44:28Z

Thanks! I'm so happy! :D

glemaitre · 2021-08-31T07:49:17Z

Nice. Thanks @lucyleeow

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lucyleeow added 2 commits June 4, 2020 10:12

wip

3420a3d

calib curve

17dd94a

github-actions bot added the module:metrics label Jun 4, 2020

lint

2f34245

NicolasHug reviewed Jun 4, 2020

View reviewed changes

sklearn/metrics/_plot/calibration_curve.py Outdated Show resolved Hide resolved

sklearn/metrics/_plot/calibration_curve.py Outdated Show resolved Hide resolved

glemaitre self-assigned this Jun 4, 2020

lucyleeow added 3 commits June 4, 2020 14:28

avoid circular import

56eda05

circ import

f98169e

circ import

5d3ea22

glemaitre removed their assignment Jun 4, 2020

glemaitre reviewed Jun 4, 2020

View reviewed changes

lucyleeow added 4 commits June 4, 2020 18:20

add tests

549106b

suggestions

12b6704

lint, fix tests

102f826

lint

cbc73ab

lucyleeow added 11 commits June 5, 2020 13:03

add brier score

8427385

update example

7beb99d

amend brier label

e0fcff6

typo

32efb16

brier var name

58caff9

lint

92a4387

update comp cal

3267309

lint, add ref link

5f3660e

minor formatting

14e52ac

plt

ceba3de

add y prob att

1196d2a

typo in test

e3797b6

Lucy Liu added 2 commits July 22, 2021 17:10

fix error msg

ecd47b9

fix error msg

e18419d

lorentzenchr added this to the 1.0 milestone Aug 23, 2021

glemaitre reviewed Aug 30, 2021

View reviewed changes

doc/whats_new/v1.0.rst Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into IS/8425

244a8fd

ogrisel reviewed Aug 30, 2021

View reviewed changes

sklearn/calibration.py Outdated Show resolved Hide resolved

ogrisel approved these changes Aug 30, 2021

View reviewed changes

examples/calibration/plot_calibration_curve.py Show resolved Hide resolved

Lucy Liu added 5 commits August 31, 2021 14:22

Merge branch 'main' into IS/8425

99f38ba

Merge branch 'IS/8425' of github.com:lucyleeow/scikit-learn into IS/8425

ec4764d

suggestions

81d43d6

update msg

66851d9

update msg

b25f03b

lucyleeow commented Aug 31, 2021

View reviewed changes

ogrisel merged commit da36f72 into scikit-learn:main Aug 31, 2021

lucyleeow deleted the IS/8425 branch August 31, 2021 07:44

samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021

ENH Add CalibrationDisplay plotting class (scikit-learn#17443)

122cd5d

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

lucyleeow mentioned this pull request Feb 16, 2022

Deprecate normalize parameter in calibration_curve #22482

Closed

		f"{classification_error} fit on multiclass ({y_pred_shape} classes)"
		" data"

Uh oh!

Conversation

lucyleeow commented Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre Jun 4, 2020

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jun 4, 2020

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jun 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lucyleeow commented Jun 4, 2020

Uh oh!

NicolasHug commented Jun 4, 2020

Uh oh!

lucyleeow commented Jun 4, 2020

Uh oh!

lucyleeow commented Jun 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucyleeow commented Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glemaitre commented Jul 22, 2021

Uh oh!

glemaitre commented Jul 22, 2021

Uh oh!

lucyleeow commented Jul 28, 2021

Uh oh!

lorentzenchr commented Aug 15, 2021

Uh oh!

lorentzenchr commented Aug 23, 2021

Uh oh!

lucyleeow commented Aug 23, 2021

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glemaitre commented Aug 30, 2021

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lucyleeow Aug 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel Aug 31, 2021

Choose a reason for hiding this comment

Uh oh!

lucyleeow commented Aug 31, 2021

Uh oh!

ogrisel commented Aug 31, 2021

Uh oh!

lucyleeow commented Aug 31, 2021

Uh oh!

lucyleeow commented Jun 4, 2020 •

edited

Loading

lucyleeow Jun 4, 2020 •

edited

Loading

lucyleeow commented Jun 5, 2020 •

edited

Loading

lucyleeow commented Jul 22, 2021 •

edited

Loading

lucyleeow Aug 31, 2021 •

edited

Loading