8000 feat(skore)!: Change the API `project.reports.metadata` to `project.summarize` by thomass-dev · Pull Request #1801 · probabl-ai/skore · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(skore)!: Change the API project.reports.metadata to project.summarize #1801

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

Merged
merged 7 commits into from
Jun 6, 2025

Conversation

thomass-dev
Copy link
Collaborator

Closes #1795.

Copy link
Contributor
github-actions bot commented Jun 5, 2025

Coverage

Coverage Report for skore/
FileStmtsMissCoverMissing
venv/lib/python3.13/site-packages/skore
   __init__.py230100% 
   _config.py280100% 
   exceptions.py440%4, 15, 19, 23
venv/lib/python3.13/site-packages/skore/project
   __init__.py20100% 
   project.py490100% 
   summary.py670100% 
   widget.py138596%375–377, 447–448
venv/lib/python3.13/site-packages/skore/sklearn
   __init__.py70100% 
   _base.py1691491%45, 58, 126, 129, 182, 185–186, 188–191, 224, 227–228
   find_estimators.py270100% 
   find_ml_task.py610100% 
   types.py26196%26
venv/lib/python3.13/site-packages/skore/sklearn/_comparison
   __init__.py50100% 
   metrics_accessor.py198298%170, 1283
   report.py950100% 
   utils.py550100% 
venv/lib/python3.13/site-packages/skore/sklearn/_cross_validation
   __init__.py50100% 
   metrics_accessor.py2070100% 
   report.py125199%480
venv/lib/python3.13/site-packages/skore/sklearn/_estimator
   __init__.py70100% 
   feature_importance_accessor.py143298%216–217
   metrics_accessor.py382997%162, 191, 193, 200, 291, 360, 364, 379, 414
   report.py166298%454–455
venv/lib/python3.13/site-packages/skore/sklearn/_plot
   __init__.py20100% 
   base.py50100% 
   style.py280100% 
   utils.py118595%50, 74–76, 80
venv/lib/python3.13/site-packages/skore/sklearn/_plot/metrics
   __init__.py50100% 
   confusion_matrix.py69494%90, 98, 120, 228
   precision_recall_curve.py230199%716
   prediction_error.py186398%177, 184, 638
   roc_curve.py242498%380, 497, 598, 791
venv/lib/python3.13/site-packages/skore/sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py490100% 
venv/lib/python3.13/site-packages/skore/sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py17194%80
   high_class_imbalance_warning.py180100% 
   random_state_unset_warning.py100100% 
   shuffle_true_warning.py10190%46
   stratify_is_set_warning.py100100% 
   time_based_column_warning.py21195%73
   train_test_split_warning.py40100% 
venv/lib/python3.13/site-packages/skore/utils
   __init__.py6266%8, 13
   _accessor.py52296%67, 108
   _environment.py270100% 
   _fixes.py80100% 
   _index.py50100% 
   _logger.py22481%15–17, 19
   _measure_time.py100100% 
   _parallel.py38392%23, 33, 124
   _patch.py13561%21, 23–24, 35, 37
   _progress_bar.py450100% 
   _show_versions.py33293%65–66
   _testing.py370100% 
TOTAL33177897% 

Tests Skipped Failures Errors Time
833 5 💤 0 ❌ 0 🔥 6m 44s ⏱️

@thomass-dev thomass-dev changed the title feat(skore): Change the API project.reports.metadata to project.summary feat(skore)!: Change the API project.reports.metadata to project.summary Jun 5, 2025
Copy link
Contributor
github-actions bot commented Jun 5, 2025

Documentation preview @ 9d090da

@glemaitre
Copy link
Member

It came to my mind that a verb would be better suited than a noun since it is a function. So, project.summarize() would be better.

So maybe we can slightly think a bit more about the naming to know if summarize is really the intended action that we perform to get the metadata. I think it is not bad I'll just take a couple of minutes to think of alternative.

@thomass-dev
Copy link
Collaborator Author
thomass-dev commented Jun 6, 2025

It came to my mind that a verb would be better suited than a noun since it is a function.

You're true! Totally agree. def summarize(self) -> Summary looks good.
Waiting for your return before making the change.

@glemaitre
Copy link
Member

In short, I have a feeling that summarize would trigger a computation to be able to do the summary while stocktake and Stock is just a listing.

Basically, since @auguste-probabl is a native speaker, I would take the one that he finds the most appropriate and we move on with it. I am already happy with both suggestions compared to the current state.

@glemaitre glemaitre changed the title feat(skore)!: Change the API project.reports.metadata to project.summary feat(skore)!: Change the API project.reports.metadata to project.summarize Jun 6, 2025
glemaitre
glemaitre previously approved these changes Jun 6, 2025
@thomass-dev thomass-dev force-pushed the skore-remove-project-reports-namespace branch from a1642f8 to 9d090da Compare June 6, 2025 09:37
@thomass-dev
Copy link
Collaborator Author
thomass-dev commented Jun 6, 2025

Ready to merge @glemaitre (do not update with main; use the merge queue 😄 ).

@auguste-probabl auguste-probabl added this pull request to the merge queue Jun 6, 2025
Merged via the queue into main with commit f1a97a2 Jun 6, 2025
31 checks passed
@auguste-probabl auguste-probabl deleted the skore-remove-project-reports-namespace branch June 6, 2025 14:48
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.

Revisit the API of the skore.Metadata
3 participants
0