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

docs(future feat): Example driven development #1239

Closed
wants to merge 3 commits into from
Closed

Conversation

MarieSacksick
Copy link
Contributor

No description provided.

@MarieSacksick MarieSacksick marked this pull request as draft January 27, 2025 18:52
@MarieSacksick MarieSacksick changed the title docs(future feat): example driven development docs(future feat): Example driven development Jan 27, 2025
Copy link
Contributor

github-actions bot commented Jan 27, 2025

Documentation preview @ 36b1e60

@auguste-probabl
Copy link
Contributor

This can be closed in favor of #1286

@auguste-probabl auguste-probabl deleted the compare branch February 14, 2025 12:06
thomass-dev added a commit that referenced this pull request Feb 19, 2025
#1286)

- [x] Rename to `ComparisonReport`
- [x] Rebase on top of #1239 and adapt
- [x] Raise if `report.metrics.accuracy(data_source="train")` is called
with at least one EstimatorReport that does not have training data
- [x] Test
- [x] Docstrings
    - [x] MetricsAccessor
- [x] Move index column "#0" in front of each metric
- [x] Pass report names in comparator
- [ ]  ~Update plots legend~ see #1309 
- The actual `RocCurveDisplay` needs a full refactor to be splitted by
use-case: estimator report, cross-validation report and finally
comparison report. In each of these use-cases, there is two scenarios
with binary classification and multi-class classification. Otherwise, it
will be unmaintainable.
- [ ] ~Investigate missing metrics in `report_metrics`~ **(deferred to
future PR)**
- The logic is split between `report_metrics` and `available_if`; it
should be merged (ideally everything in `available_if`?)
- [ ] ~Refactor to make `CrossValidationReport` depend on it~
**(deferred to future PR)**
- [x] ~Change EstimatorReport `repr`?~ Issue
#1293

Closes #1245 

Co-authored-by: Auguste <[email protected]>
Co-authored-by: Sylvain Combettes <[email protected]>
glemaitre pushed a commit to glemaitre/skore that referenced this pull request Feb 20, 2025
probabl-ai#1286)

- [x] Rename to `ComparisonReport`
- [x] Rebase on top of probabl-ai#1239 and adapt
- [x] Raise if `report.metrics.accuracy(data_source="train")` is called
with at least one EstimatorReport that does not have training data
- [x] Test
- [x] Docstrings
    - [x] MetricsAccessor
- [x] Move index column "#0" in front of each metric
- [x] Pass report names in comparator
- [ ]  ~Update plots legend~ see probabl-ai#1309 
- The actual `RocCurveDisplay` needs a full refactor to be splitted by
use-case: estimator report, cross-validation report and finally
comparison report. In each of these use-cases, there is two scenarios
with binary classification and multi-class classification. Otherwise, it
will be unmaintainable.
- [ ] ~Investigate missing metrics in `report_metrics`~ **(deferred to
future PR)**
- The logic is split between `report_metrics` and `available_if`; it
should be merged (ideally everything in `available_if`?)
- [ ] ~Refactor to make `CrossValidationReport` depend on it~
**(deferred to future PR)**
- [x] ~Change EstimatorReport `repr`?~ Issue
probabl-ai#1293

Closes probabl-ai#1245 

Co-authored-by: Auguste <[email protected]>
Co-authored-by: Sylvain Combettes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants