Conversation
component_explained_x_variance and explained_x_variance had diverging shape-check logic; align them on the same 2D/shape validation and trim redundant docstrings.
fit() was one long method and predict/transform/score_distance/ q_residuals each re-derived Xs/X_filtered/scores via separate, overlapping reconstruction helpers. Introduce _OPLSProjection and _project_validated() as the single path for scaling + orthogonal filtering + predictive transform, and split fit() into named steps. Update OPLSDA to go through the new projection helper too.
Plotting display from_estimator() silently unwrapped GridSearchCV/other search meta-estimators via best_estimator_. That special case only handled the two-attribute shape and hid which model was actually being plotted. Require callers to pass search.best_estimator_ explicitly; plotting now only accepts OPLS, OPLSDA, or a pipeline ending in one. BREAKING CHANGE: passing a fitted search meta-estimator directly to OPLSScoresDisplay.from_estimator/SPlotDisplay.from_estimator now raises TypeError.
… _stack_columns, cache _ssq in orthogonal extraction, add all-scale predict and multi-output score tests
There was a problem hiding this comment.
Pull request overview
This PR simplifies and consolidates internal validation/projection logic across the library, while tightening plotting support to require explicit base estimators (or pipelines) rather than accepting search meta-estimators directly.
Changes:
- Centralizes integer validation (including explicit bool rejection) via new
_utilshelpers and reuses them across modules. - Refactors
OPLSprojection/reconstruction paths around a single_project_validated()flow and updates plotting to consume it. - Updates plotting and docs/tests to no longer accept
GridSearchCVdirectly (requiringbest_estimator_), and tightenspermutation_testresponse-shape validation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_plotting.py | Updates plotting tests to require best_estimator_ for search wrappers and to assert clearer error behavior. |
| tests/test_o2pls.py | Adds regression tests around multi-output scoring and prediction equivalences across scaling modes. |
| src/scikit_opls/validation.py | Reuses _validate_int and enforces 1D y in permutation_test. |
| src/scikit_opls/plotting.py | Removes search-estimator unwrapping and switches score/S-plot computations to _project_validated(). |
| src/scikit_opls/_utils.py | Adds shared helpers _reject_bool_param and _validate_int. |
| src/scikit_opls/_orthogonal.py | Replaces local int validation with _validate_int and simplifies tolerance checks. |
| src/scikit_opls/_opls.py | Refactors fit() into smaller helpers and introduces _OPLSProjection / _project_validated() for shared projections. |
| src/scikit_opls/_opls_da.py | Reuses _reject_bool_param and routes prediction through _project_validated(). |
| src/scikit_opls/_o2pls.py | Reuses _reject_bool_param for bool-rejection of integral hyperparameters. |
| src/scikit_opls/_o2pls_core.py | Reuses _validate_int and simplifies internal stacking/diagnostic calculations and docstrings. |
| src/scikit_opls/_inspection.py | Tightens shape checks and simplifies explained-variance computations for fitted arrays. |
| README.md | Updates plotting guidance to require explicit search.best_estimator_ for tuned models. |
| docs/quickstart.md | Mirrors README plotting guidance change for tuned models (best_estimator_). |
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
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.
Summary
Checklist
parametrize_with_checks(tests/test_sklearn_compat.py) still passesuv run ruff checkanduv run ruff format --checkpassuv run pyright srcpassesCHANGES.mdPublic-API impact