FIX use same API for CalibrationDisplay than other Display #21031
Conversation
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
| name : str, default=None | ||
| Name for labeling curve. | ||
| estimator_name : str, default=None | ||
| Name of estimator. If None, the estimator name is not shown. |
There was a problem hiding this comment.
nitpick:
| Name of estimator. If None, the estimator name is not shown. | |
| Name of estimator. If `None`, the estimator name is not shown. |
There was a problem hiding this comment.
To be pedantic there is no label shown in the plot only if self.estimator is None and in the plot method name is also None...?
| @@ -1061,8 +1062,7 @@ def plot(self, *, ax=None, name=None, ref_line=True, **kwargs): | |||
| if ax is None: | |||
| fig, ax = plt.subplots() | |||
|
|
|||
| name = self.name if name is None else name | |||
| self.name = name | |||
There was a problem hiding this comment.
Agree that this is weird, maybe it's left over from a previous iteration/API and I forgot to change.
sklearn/calibration.py
Outdated
| Name for labeling curve. If `None`, the name of the estimator | ||
| is used. |
There was a problem hiding this comment.
This is only true if from_estimator is used. If from_predictions is used the default is 'Classifier' (with @thomasjpfan's new suggestion). But since this is not public facing API, maybe we can just say if None a 'default' value is used...?
There was a problem hiding this comment.
It is even different. If name is None, then we use estimator_name and if it is also None then we don't show anything.
|
tagging #20965 |
…arn#21031) * FIX use same API for CalibrationDisplay than other Display * Update sklearn/calibration.py Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> * iter Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
* FIX use same API for CalibrationDisplay than other Display * Update sklearn/calibration.py Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> * iter Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
…arn#21031) * FIX use same API for CalibrationDisplay than other Display * Update sklearn/calibration.py Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> * iter Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
closes #21027
Use
estimator_nameinstead ofnameas an attribute in the display forCalibrationDisplayping @thomasjpfan