refactor(skore): Modify dataframe format of MetricsSummaryDisplay#1839
Draft
glemaitre wants to merge 8 commits intoprobabl-ai:mainfrom
Draft
refactor(skore): Modify dataframe format of MetricsSummaryDisplay#1839glemaitre wants to merge 8 commits intoprobabl-ai:mainfrom
glemaitre wants to merge 8 commits intoprobabl-ai:mainfrom
Conversation
Collaborator
|
Superseded by #2322 ? Please close if yes |
6 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 6, 2026
….frame()` (#2536) <!-- 🙌 Thanks for contributing a pull request! If this is your first contribution, take a look at our contribution guidelines: https://docs.skore.probabl.ai/dev/contributing.html --> #### Change description <!-- Please describe what your contribution changes for skore. Here is some inspiration for what to write here: - Are you adding a new feature? Fixing a bug? - Can you give an example of your change in action, e.g. a snippet of code or a plot? - Is your change likely to break users' code? - Are there any other details the reviewer should be aware of, such as API design choices, performance characteristics or edge cases? Please reference issues/PRs when possible, e.g. "Fixes #1234", "Closes #3456", "See also #7890". More information [here](https://github.com/blog/1506-closing-issues-via-pull-requests). --> `EstimatorReport.metrics.summarize()` no longer accepts arguments `flat_index` or `favorability`. These have been moved to `MetricsSummaryDisplay.frame()`. This moves the responsibility of displaying things from `EstimatorReport._MetricsAccessor` to `MetricsSummaryDisplay`. ```python # Before report.metrics.summarize(flat_index=True, favorability=True).frame() # After report.metrics.summarize().frame(flat_index=True, favorability=True) ``` This is a breaking change. The rest of the PR consists in various refactorings, in particular the tests have been updated to reflect the change in responsibility: - Tests of `summarize()` were extracted from `tests/unit/reports/estimator/metrics/test_numeric.py` to a new file, `tests/unit/reports/estimator/metrics/test_summarize.py`. - `test_summarize.py` now tests that `summarize()` behaves well, and that the output of `summarize()` has a well-formed DataFrame. `.frame()` is never called. - `displays/metrics_summary/test_estimator.py` has been rewritten to specifically test `.frame()` arguments. A number of commits are included that could be pulled out into a separate PR if needed. Closes #2533 Supersedes #1839 in part #### Contribution checklist <!-- Below are some of the criteria that the review will include. Feel free to use it as a checklist to ensure that your contribution is high-quality. --> - [x] Unit tests were added or updated (if necessary) - [x] Documentation was added or updated (if necessary) #### AI usage disclosure <!-- If AI tools were involved in creating this PR, please check all boxes that apply below and make sure you understand our [automated contributions policy](https://docs.skore.probabl.ai/dev/contributing.html#automated-contributions-policy) --> AI tools were involved for: - [ ] Code generation (e.g., when writing an implementation or fixing a bug) - [x] Test/benchmark generation - [ ] Documentation (including examples) - [ ] Research and understanding In particular I used Claude to increase coverage. <!-- Any other comments can go here. Thanks again for contributing! -->
direkkakkar319-ops
pushed a commit
to direkkakkar319-ops/skore_05
that referenced
this pull request
Mar 8, 2026
….frame()` (probabl-ai#2536) <!-- 🙌 Thanks for contributing a pull request! If this is your first contribution, take a look at our contribution guidelines: https://docs.skore.probabl.ai/dev/contributing.html --> #### Change description <!-- Please describe what your contribution changes for skore. Here is some inspiration for what to write here: - Are you adding a new feature? Fixing a bug? - Can you give an example of your change in action, e.g. a snippet of code or a plot? - Is your change likely to break users' code? - Are there any other details the reviewer should be aware of, such as API design choices, performance characteristics or edge cases? Please reference issues/PRs when possible, e.g. "Fixes probabl-ai#1234", "Closes #3456", "See also #7890". More information [here](https://github.com/blog/1506-closing-issues-via-pull-requests). --> `EstimatorReport.metrics.summarize()` no longer accepts arguments `flat_index` or `favorability`. These have been moved to `MetricsSummaryDisplay.frame()`. This moves the responsibility of displaying things from `EstimatorReport._MetricsAccessor` to `MetricsSummaryDisplay`. ```python # Before report.metrics.summarize(flat_index=True, favorability=True).frame() # After report.metrics.summarize().frame(flat_index=True, favorability=True) ``` This is a breaking change. The rest of the PR consists in various refactorings, in particular the tests have been updated to reflect the change in responsibility: - Tests of `summarize()` were extracted from `tests/unit/reports/estimator/metrics/test_numeric.py` to a new file, `tests/unit/reports/estimator/metrics/test_summarize.py`. - `test_summarize.py` now tests that `summarize()` behaves well, and that the output of `summarize()` has a well-formed DataFrame. `.frame()` is never called. - `displays/metrics_summary/test_estimator.py` has been rewritten to specifically test `.frame()` arguments. A number of commits are included that could be pulled out into a separate PR if needed. Closes probabl-ai#2533 Supersedes probabl-ai#1839 in part #### Contribution checklist <!-- Below are some of the criteria that the review will include. Feel free to use it as a checklist to ensure that your contribution is high-quality. --> - [x] Unit tests were added or updated (if necessary) - [x] Documentation was added or updated (if necessary) #### AI usage disclosure <!-- If AI tools were involved in creating this PR, please check all boxes that apply below and make sure you understand our [automated contributions policy](https://docs.skore.probabl.ai/dev/contributing.html#automated-contributions-policy) --> AI tools were involved for: - [ ] Code generation (e.g., when writing an implementation or fixing a bug) - [x] Test/benchmark generation - [ ] Documentation (including examples) - [ ] Research and understanding In particular I used Claude to increase coverage. <!-- Any other comments can go here. Thanks again for contributing! -->
direkkakkar319-ops
pushed a commit
to direkkakkar319-ops/skore_05
that referenced
this pull request
Mar 9, 2026
….frame()` (probabl-ai#2536) <!-- 🙌 Thanks for contributing a pull request! If this is your first contribution, take a look at our contribution guidelines: https://docs.skore.probabl.ai/dev/contributing.html --> #### Change description <!-- Please describe what your contribution changes for skore. Here is some inspiration for what to write here: - Are you adding a new feature? Fixing a bug? - Can you give an example of your change in action, e.g. a snippet of code or a plot? - Is your change likely to break users' code? - Are there any other details the reviewer should be aware of, such as API design choices, performance characteristics or edge cases? Please reference issues/PRs when possible, e.g. "Fixes probabl-ai#1234", "Closes #3456", "See also #7890". More information [here](https://github.com/blog/1506-closing-issues-via-pull-requests). --> `EstimatorReport.metrics.summarize()` no longer accepts arguments `flat_index` or `favorability`. These have been moved to `MetricsSummaryDisplay.frame()`. This moves the responsibility of displaying things from `EstimatorReport._MetricsAccessor` to `MetricsSummaryDisplay`. ```python # Before report.metrics.summarize(flat_index=True, favorability=True).frame() # After report.metrics.summarize().frame(flat_index=True, favorability=True) ``` This is a breaking change. The rest of the PR consists in various refactorings, in particular the tests have been updated to reflect the change in responsibility: - Tests of `summarize()` were extracted from `tests/unit/reports/estimator/metrics/test_numeric.py` to a new file, `tests/unit/reports/estimator/metrics/test_summarize.py`. - `test_summarize.py` now tests that `summarize()` behaves well, and that the output of `summarize()` has a well-formed DataFrame. `.frame()` is never called. - `displays/metrics_summary/test_estimator.py` has been rewritten to specifically test `.frame()` arguments. A number of commits are included that could be pulled out into a separate PR if needed. Closes probabl-ai#2533 Supersedes probabl-ai#1839 in part #### Contribution checklist <!-- Below are some of the criteria that the review will include. Feel free to use it as a checklist to ensure that your contribution is high-quality. --> - [x] Unit tests were added or updated (if necessary) - [x] Documentation was added or updated (if necessary) #### AI usage disclosure <!-- If AI tools were involved in creating this PR, please check all boxes that apply below and make sure you understand our [automated contributions policy](https://docs.skore.probabl.ai/dev/contributing.html#automated-contributions-policy) --> AI tools were involved for: - [ ] Code generation (e.g., when writing an implementation or fixing a bug) - [x] Test/benchmark generation - [ ] Documentation (including examples) - [ ] Research and understanding In particular I used Claude to increase coverage. <!-- Any other comments can go here. Thanks again for contributing! -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1837
This PR intends to:
.summarize.frameis converting from long to wide format when necessaryframeto provide flexibility