y_test, y_true args removed from roc constructor #135

yafimvo · 2022-12-25T11:58:54Z

Describe your changes

y_test, y_true args removed from roc constructor
Generates roc using fpr and tpr values
ROC and ROCAdd implemented with AbstractPlot and AbstractComposedPlot
Examples updated to roc.from_raw_data(y_true, y_score)
Old API invokes roc.from_raw_data(y_true, y_score, ax=ax)

Issue ticket number and link

Closes #121

Checklist before requesting a review

I have performed a self-review of my code
I have added thorough tests (when necessary).
I have added the right documentation (when needed). Product update? If yes, write one line about this update.

coveralls · 2022-12-25T12:03:22Z

Pull Request Test Coverage Report for Build 3902708281

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

96 of 97 (98.97%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.05%) to 94.629%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/sklearn_evaluation/plot/roc.py	95	96	98.96%

Totals
Change from base Build 3902679656:	0.05%
Covered Lines:	2643
Relevant Lines:	2793

💛 - Coveralls

src/sklearn_evaluation/plot/roc.py

idomic · 2023-01-05T14:06:09Z

@yafimvo please address

…_values_removed_from_roc_init

yafimvo · 2023-01-10T22:53:26Z

@idomic let's move this conversation here.

I don't think we need to combine issue #98 with this since this one has a lot of changes.

idomic

looks good to me

idomic · 2023-01-11T16:44:14Z

Is there documentation? That's the only thing I was missing.
Good test cases!

@edublancas if there's any comments now's the time.

idomic · 2023-01-11T17:42:28Z

@yafimvo I also don't see a changelog entry.

edublancas · 2023-01-11T18:17:50Z

src/sklearn_evaluation/plot/roc.py

 from sklearn_evaluation import __version__
 import json
 from pathlib import Path
-from warnings import warn  # noqa
+from ..plot.plot import AbstractPlot, AbstractComposedPlot


we're no longer using relative imports, please change

fixed.

also changed it in src/sklearn_evaluation/plot/classification.py

edublancas · 2023-01-11T18:25:57Z

src/sklearn_evaluation/plot/roc.py

-    For example, if ``y_true = [2, 2, 1, 0, 0, 1, 2]``, then the
-    first column in y_score must contain the scores for class 0,
-    second column for class 1 and so on.
-
    Examples
    --------
    .. plot:: ../examples/roc_new_api.py


let's update this example so it shows how the new API works (using from_raw_data).

roc for binary classification

roc for multi-class

comparing two binary classifiers (plot1 + plot2)

We already had these examples (except the multi-class).
I changed the names and added an example for multi-class.

please check if these examples are ok.

@idomic Does this multi-class example address #98 ?

Not really, the issue there is the error message.

…_values_removed_from_roc_init

Replaced n_clusters to range_n_clusters in plot.elbow_curve (ploomber#182) * range_n_clusters added to elbow_curve signature * reversed auto lint changes * deprecation warning added to docs * deprecation note fixed Co-authored-by: yafim <[email protected]> Updating ROC constructor, removed y_test, y_true args (ploomber#135) * y_test, y_true args removed from roc constructor * AbstractPlot added to ROC * roc_rates_n_classes removed * tests added, code cleaned * examples added, relative import fixed, changelog updated Co-authored-by: yafim <[email protected]> Co-authored-by: Ido M <[email protected]> baseline image updated changelog updated

Replaced n_clusters to range_n_clusters in plot.elbow_curve (#182) * range_n_clusters added to elbow_curve signature * reversed auto lint changes * deprecation warning added to docs * deprecation note fixed Co-authored-by: yafim <[email protected]> Updating ROC constructor, removed y_test, y_true args (#135) * y_test, y_true args removed from roc constructor * AbstractPlot added to ROC * roc_rates_n_classes removed * tests added, code cleaned * examples added, relative import fixed, changelog updated Co-authored-by: yafim <[email protected]> Co-authored-by: Ido M <[email protected]> baseline image updated changelog updated Co-authored-by: yafim <[email protected]>

y_test, y_true args removed from roc constructor

351603a

idomic requested a review from edublancas December 26, 2022 02:08

edublancas suggested changes Dec 26, 2022

View reviewed changes

src/sklearn_evaluation/plot/roc.py Outdated Show resolved Hide resolved

src/sklearn_evaluation/plot/roc.py Outdated Show resolved Hide resolved

yafim added 5 commits January 9, 2023 17:10

Merge branch 'master' of github.com:yafimvo/sklearn-evaluation into y…

155d981

…_values_removed_from_roc_init

AbstractPlot added to ROC

6991c89

roc_rates_n_classes removed

0994497

tests added, code cleaned

7d3f11e

Merge branch 'master' of github.com:yafimvo/sklearn-evaluation into y…

c54cd68

…_values_removed_from_roc_init

yafimvo requested review from edublancas and idomic January 10, 2023 22:59

idomic previously approved these changes Jan 11, 2023

View reviewed changes

edublancas suggested changes Jan 11, 2023

View reviewed changes

yafim added 2 commits January 12, 2023 09:57

examples added, relative import fixed, changelog updated

74919f6

Merge branch 'master' of github.com:yafimvo/sklearn-evaluation into y…

1c0f81c

…_values_removed_from_roc_init

yafimvo dismissed idomic’s stale review via 1c0f81c January 12, 2023 08:14

Merge branch 'master' into y_values_removed_from_roc_init

1c303d8

idomic merged commit f887dcf into ploomber:master Jan 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

y_test, y_true args removed from roc constructor #135

y_test, y_true args removed from roc constructor #135

yafimvo commented Dec 25, 2022 •

edited

Loading

coveralls commented Dec 25, 2022 •

edited

Loading

idomic commented Jan 5, 2023

yafimvo commented Jan 10, 2023

idomic left a comment

idomic commented Jan 11, 2023

idomic commented Jan 11, 2023

edublancas Jan 11, 2023

yafimvo Jan 12, 2023 •

edited

Loading

edublancas Jan 11, 2023

yafimvo Jan 12, 2023

idomic Jan 12, 2023

y_test, y_true args removed from roc constructor #135

y_test, y_true args removed from roc constructor #135

Conversation

yafimvo commented Dec 25, 2022 • edited Loading

Describe your changes

Issue ticket number and link

Checklist before requesting a review

coveralls commented Dec 25, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3902708281

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

idomic commented Jan 5, 2023

yafimvo commented Jan 10, 2023

idomic left a comment

Choose a reason for hiding this comment

idomic commented Jan 11, 2023

idomic commented Jan 11, 2023

edublancas Jan 11, 2023

Choose a reason for hiding this comment

yafimvo Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

edublancas Jan 11, 2023

Choose a reason for hiding this comment

yafimvo Jan 12, 2023

Choose a reason for hiding this comment

idomic Jan 12, 2023

Choose a reason for hiding this comment

yafimvo commented Dec 25, 2022 •

edited

Loading

coveralls commented Dec 25, 2022 •

edited

Loading

yafimvo Jan 12, 2023 •

edited

Loading