Skip to content

ENH add from_cv_results in RocCurveDisplay (Multi-display) #30359

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

Closed
wants to merge 3 commits into from

Conversation

lucyleeow
Copy link
Member

@lucyleeow lucyleeow commented Nov 28, 2024

Reference Issues/PRs

Supercedes #25939

This is part of a group of draft PRs to determine best API for adding plots for cv results to our displays.

For all 3 options we take the output of cross_validate, and use the fitted estimator and test indicies. No fitting is done in the display.

We do recalculate the predictions (which would have already been done in cross_validate), which could be avoided if we decided to change cross_validate to optionally return the predictions as well (note this would make cross_val_predict redundant).
See more thread: #25939 (comment)). I think should be outside of the scope of this body of work though.

What does this implement/fix? Explain your changes.

Essentially updates #25939 to main and removes the average and variation parts.

With this approach we have to create a new Multi...Display class for each display. These could be useful to users who want to plot curves that don't come cv but if they are plotting from cv we discourage them to use this class directly (and use from_cv_results instead)

Any other comments?

Plot looks like:
image

zip(cv_results["estimator"], cv_results["indices"]["test"], fold_name_)
):
y_true = _safe_indexing(y, test_indices)
y_pred = _get_response_values_binary(
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 could have used _validate_and_get_response_values instead here. I decided against because it does other checking that we don't need here but 🤷

Copy link

github-actions bot commented Nov 28, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: bd66360. Link to the linter CI: here

@lucyleeow lucyleeow marked this pull request as draft November 28, 2024 05:15
@lucyleeow
Copy link
Member Author

closing in favour of #30399

@lucyleeow lucyleeow closed this Jan 22, 2025
@lucyleeow lucyleeow deleted the cv_results1 branch February 7, 2025 09:35
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.

1 participant