Skip to content

Conversation

@glemaitre
Copy link
Member

@glemaitre glemaitre commented Jan 13, 2025

It is a refactor to ease the review in #1091.

It is only moving the file from sklearn/_estimator/base.py to sklearn/_base.py since some base class will be in common between the EstimatorReport and CrossValidationReport.

In addition, I factor out an additional base class link to the help of the *Report class and the _get_cached_response_values that will also be used in the CrossValidationReport.

So in short, there is no new code here.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Coverage

Coverage Report for backend
FileStmtsMissCoverMissing
venv/lib/python3.12/site-packages/skore
   __init__.py120100% 
   __main__.py8180%19
   exceptions.py30100% 
venv/lib/python3.12/site-packages/skore/cli
   __init__.py50100% 
   cli.py33385%104, 111, 117
   color_format.py43390%35–>40, 41–43
   launch_dashboard.py261539%36–57
   quickstart_command.py14750%37–51
venv/lib/python3.12/site-packages/skore/item
   __init__.py210100% 
   cross_validation_item.py1371093%27–42, 370
   item.py411368%85, 88, 92–112
   item_repository.py42293%12–13
   media_item.py70494%15–18
   numpy_array_item.py25193%15
   pandas_dataframe_item.py34195%15
   pandas_series_item.py34195%15
   polars_dataframe_item.py32194%15
   polars_series_item.py27194%15
   primitive_item.py27292%13–15
   sklearn_base_estimator_item.py33195%15
   skrub_table_report_item.py10186%11
venv/lib/python3.12/site-packages/skore/persistence
   __init__.py00100% 
   abstract_storage.py22195%130
   disk_cache_storage.py33195%44
   in_memory_storage.py200100% 
venv/lib/python3.12/site-packages/skore/project
   __init__.py30100% 
   create.py52888%116–122, 132–133, 140–141
   load.py23389%43–45
   open.py140100% 
   project.py64491%135, 149, 183, 187
venv/lib/python3.12/site-packages/skore/sklearn
   __init__.py40100% 
   _base.py141298%168–>173, 184–185
   find_ml_task.py35194%41–>49, 47–>49, 50
   types.py20100% 
venv/lib/python3.12/site-packages/skore/sklearn/_estimator
   __init__.py100100% 
   metrics_accessor.py198298%131, 267
   report.py109098%160–>166, 168–>170
   utils.py11110%1–19
venv/lib/python3.12/site-packages/skore/sklearn/_plot
   __init__.py40100% 
   precision_recall_curve.py126297%200–>203, 313–314
   prediction_error.py75099%289–>297
   roc_curve.py95394%156, 167–>170, 223–224
   utils.py770100% 
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation
   __init__.py20100% 
   cross_validation_helpers.py47490%104–>136, 123–126
   cross_validation_reporter.py35195%177
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation/plots
   __init__.py00100% 
   compare_scores_plot.py29192%10, 45–>48
   timing_plot.py29194%10
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py36294%16–17
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py17378%16–18, 80
   high_class_imbalance_warning.py18288%16–18
   random_state_unset_warning.py11187%15
   shuffle_true_warning.py9091%44–>exit
   stratify_is_set_warning.py11187%15
   time_based_column_warning.py22189%17, 69–>exit
   train_test_split_warning.py5180%21
venv/lib/python3.12/site-packages/skore/ui
   __init__.py00100% 
   app.py25571%24, 53–58
   dependencies.py7186%12
   project_routes.py500100% 
venv/lib/python3.12/site-packages/skore/utils
   __init__.py00100% 
   _accessor.py70100% 
   _logger.py21484%14–18
   _show_versions.py310100% 
venv/lib/python3.12/site-packages/skore/view
   __init__.py00100% 
   view.py50100% 
   view_repository.py16283%8–9
TOTAL223613593% 

Tests Skipped Failures Errors Time
398 0 💤 0 ❌ 0 🔥 46.762s ⏱️

thomass-dev
thomass-dev previously approved these changes Jan 14, 2025
@thomass-dev
Copy link
Collaborator

Can you resolve the conflict @glemaitre before i merge? Thanks.

@glemaitre
Copy link
Member Author

Let me put the PR in draft for a couple of minutes, the time to move one test related to the base class.

@glemaitre
Copy link
Member Author

I'll ping when ready

@glemaitre glemaitre marked this pull request as draft January 14, 2025 08:30
@glemaitre glemaitre marked this pull request as ready for review January 14, 2025 09:24
@glemaitre
Copy link
Member Author

OK ready for review. I adapted one integration test to a unit tests. It should be fine now.

@thomass-dev thomass-dev merged commit 4d28474 into probabl-ai:main Jan 14, 2025
14 of 15 checks passed
waridrox pushed a commit to waridrox/skore that referenced this pull request Apr 15, 2025
It is a refactor to ease the review in probabl-ai#1091.


It is only moving the file from `sklearn/_estimator/base.py` to
`sklearn/_base.py` since some base class will be in common between the
`EstimatorReport` and `CrossValidationReport`.

In addition, I factor out an additional base class link to the help of
the `*Report` class and the `_get_cached_response_values` that will also
be used in the `CrossValidationReport`.

So in short, there is no new code here.
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