docs: Use skore for evaluation models#1170
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 skore and skrub missing from pixi docs feature dependencies
skore was added to [project.optional-dependencies] docs at pyproject.toml:52, but it was NOT added to [tool.pixi.feature.docs.dependencies] at pyproject.toml:113-123. The pixi docs environment is used for build-docs task (pyproject.toml:207). If the pixi environment doesn't pull in skore through some other mechanism (e.g. pypi-dependencies from the editable install's extras), the doc build under pixi could fail. The same applies to skrub.
(Refers to lines 113-123)
Was this helpful? React with 👍 or 👎 to provide feedback.
| results.rename( | ||
| { | ||
| previous_name: new_name | ||
| for previous_name, (new_name, _) in zip( | ||
| results.columns.get_level_values(1), estimators | ||
| ) | ||
| }, | ||
| axis="columns", | ||
| level=1, | ||
| ) |
There was a problem hiding this comment.
🟡 Column renaming in the results table has no effect because the returned value is discarded
The renamed results table is never stored back (results.rename(...) at examples/applications/plot_impact_imbalanced_classes.py:219-228), so the example displays the original auto-generated column names instead of the human-readable estimator names.
Impact: The rendered example in the documentation shows cryptic default column identifiers instead of descriptive names like "Dummy classifier" or "Logistic regression".
DataFrame.rename() returns a new object by default
pandas.DataFrame.rename() does not modify the DataFrame in place unless inplace=True is passed. The code at lines 219–228 calls results.rename(...) but discards the return value. The dictionary comprehension correctly maps old column names to new human-readable names from the estimators list, but since the result is never assigned (e.g. results = results.rename(...)) or displayed, the rename is silently lost. The cell in the rendered notebook will show nothing because the last expression in the cell is the discarded return value of rename, not results itself — but even if it were displayed, it would show the un-renamed columns.
| results.rename( | |
| { | |
| previous_name: new_name | |
| for previous_name, (new_name, _) in zip( | |
| results.columns.get_level_values(1), estimators | |
| ) | |
| }, | |
| axis="columns", | |
| level=1, | |
| ) | |
| results = results.rename( | |
| { | |
| previous_name: new_name | |
| for previous_name, (new_name, _) in zip( | |
| results.columns.get_level_values(1), estimators | |
| ) | |
| }, | |
| axis="columns", | |
| level=1, | |
| ) | |
| results |
Was this helpful? React with 👍 or 👎 to provide feedback.
| "tensorflow>=2.16.1,<3", | ||
| "matplotlib>=3.7.3,<4", | ||
| "seaborn>=0.12.2,<1", | ||
| "skore>=0.15,<1", | ||
| "memory_profiler>=0.61.0,<1", | ||
| "numpydoc>=1.5.0,<2", | ||
| "sphinx>=8.0.2,<9", |
There was a problem hiding this comment.
🔍 skrub dependency used in example but not declared in pyproject.toml
The example examples/applications/plot_impact_imbalanced_classes.py:100 imports from skrub import tabular_pipeline, but skrub is not listed anywhere in pyproject.toml — neither in [project.optional-dependencies] docs nor in [tool.pixi.feature.docs.dependencies]. It currently works because skore (which IS declared) transitively depends on skrub, but this is fragile: if skore ever drops or makes skrub optional, the doc build will break. Consider adding skrub explicitly to the docs dependencies.
(Refers to lines 47-61)
Was this helpful? React with 👍 or 👎 to provide feedback.
| fig, ax = plt.subplots(figsize=(9, 9)) | ||
| for d in disp: | ||
| d.plot(ax=ax, curve_kwargs={"linestyle": "--"}) | ||
| ax.plot([0, 1], [0, 1], linestyle="--", color="k") | ||
| ax.axis("square") | ||
| fig.suptitle("Comparison of over-sampling methods \nwith a 3NN classifier") | ||
| ax.set_xlim([0, 1]) | ||
| ax.set_ylim([0, 1]) | ||
| sns.despine(offset=10, ax=ax) | ||
| plt.legend(loc="lower right", fontsize=16) | ||
| plt.tight_layout() | ||
| for name, report in reports.items(): | ||
| report.metrics.roc().plot() | ||
| plt.show() |
There was a problem hiding this comment.
🔍 ROC curves likely plotted on separate figures instead of overlaid for comparison
At examples/applications/plot_over_sampling_benchmark_lfw.py:118, fig, ax = plt.subplots(figsize=(9, 9)) creates a figure and axes, but ax is never passed to report.metrics.roc().plot() on line 120. The original code carefully overlaid all ROC curves on a single axes for visual comparison. Depending on skore's API, each .plot() call may create its own figure, meaning the curves won't be overlaid and the created fig/ax go unused. The narrative at line 115 says "We can also plot the ROC curves for each pipeline", implying a single comparative plot was intended. This needs verification against the skore API.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
PR to use
skoreto evaluate different models in the documentation.