Surfaced during the ADR-012 refactor. lin_slope is the only metric in src/modelskill/metrics.py that requires a regression computation; every other metric is pure aggregation. As a side effect, the regression helper has to live in a shared internal module (modelskill/_utils.py:linear_regression) because both lin_slope and plotting/_scatter.py need it.
Why it's worth questioning
- Slope alone is an incomplete skill summary. A perfect slope of 1 with a non-zero intercept is still a biased model. Users wanting linearity diagnostics typically want slope + intercept + r² together.
- It's the lone cross-module pull. Without
lin_slope, the regression helper would live next to its only other user — the scatter trend line in plotting/_misc.py.
- The exposure feels incidental.
lin_slope reads more like "we already needed this for plotting, let's surface the slope as a bonus number" than like a deliberate skill metric choice.
Current usage
- Definition:
src/modelskill/metrics.py:711
- Registered with
@metric(best=1, has_units=False)
- Public docs:
docs/api/metrics.qmd:46,686
- Tested:
tests/test_aggregated_skill.py:312 (cc2.skill(metrics=[\"bias\", \"rmse\", \"lin_slope\", \"si\"]))
Options to consider
- Inline
scipy.stats.linregress inside lin_slope and move linear_regression to plotting/_misc.py. Smallest change, no public API impact, removes the last reason _utils.py holds a regression helper.
- Deprecate
lin_slope and replace with a richer regression diagnostic (e.g. Comparer.regression() returning slope/intercept/r²). Cleaner conceptually, breaking-change scope.
- Leave it. It works and is published; the cross-module helper is fine.
No action needed in #648 — this issue exists so the discussion isn't lost.
Surfaced during the ADR-012 refactor.
lin_slopeis the only metric insrc/modelskill/metrics.pythat requires a regression computation; every other metric is pure aggregation. As a side effect, the regression helper has to live in a shared internal module (modelskill/_utils.py:linear_regression) because bothlin_slopeandplotting/_scatter.pyneed it.Why it's worth questioning
lin_slope, the regression helper would live next to its only other user — the scatter trend line inplotting/_misc.py.lin_slopereads more like "we already needed this for plotting, let's surface the slope as a bonus number" than like a deliberate skill metric choice.Current usage
src/modelskill/metrics.py:711@metric(best=1, has_units=False)docs/api/metrics.qmd:46,686tests/test_aggregated_skill.py:312(cc2.skill(metrics=[\"bias\", \"rmse\", \"lin_slope\", \"si\"]))Options to consider
scipy.stats.linregressinsidelin_slopeand movelinear_regressiontoplotting/_misc.py. Smallest change, no public API impact, removes the last reason_utils.pyholds a regression helper.lin_slopeand replace with a richer regression diagnostic (e.g.Comparer.regression()returning slope/intercept/r²). Cleaner conceptually, breaking-change scope.No action needed in #648 — this issue exists so the discussion isn't lost.