feat: adopt scikit-learn API conventions for 1.0.0#91
Conversation
docs: add MkDocs documentation site and expand README
ci: switch docs deployment to GitHub Actions Pages
ci: bump Pages actions to v5 for Node.js 24 support
Move data parameters (data, distance_matrix, neighbor_matrix, cluster_labels) from __init__() to fit(), aligning with scikit-learn estimator conventions. Configuration parameters (extent, n_neighbors, use_numba, n_jobs, progress_bar) remain in the constructor. Passing data params to __init__() still works with a FutureWarning for backward compatibility. Re-fitting with new data is now supported. Add LoOP class alias and export __version__ from the package. Closes #5, Closes #7 Co-authored-by: Cursor <cursoragent@cursor.com>
Extract the monolithic loop.py (~1100 lines) into responsibility-based modules while preserving the identical public API: - exceptions.py: exception hierarchy (PyNomalyError, ValidationError, etc.) - _utils.py: Utils class (progress bar) - _validation.py: ValidationMixin + @accepts decorator - _distance.py: DistanceMixin + Numba JIT kernels - _pipeline.py: PipelineMixin (LoOP scoring math) - loop.py: slim orchestrator composing the class from mixins No API changes. All existing import patterns continue to work. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors PyNomaly’s LocalOutlierProbability estimator toward scikit-learn-style usage by moving data-bearing inputs from __init__() to fit(), while preserving backward compatibility via FutureWarnings and updating the public API/docs for the 1.0.0 release.
Changes:
- Shift data parameters (
data,distance_matrix,neighbor_matrix,cluster_labels) tofit()and addFutureWarning-based backward compatibility for passing them to__init__(). - Refactor the implementation into mixins/modules (
_validation,_distance,_pipeline,_utils) and addLoOPalias +__version__export. - Update tests and documentation/examples to the new API and bump version references to 1.0.0.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_loop.py | Updates existing tests to call fit(X) and adds new tests for compatibility/refit/alias behavior. |
| setup.py | Bumps package version to 1.0.0 and updates download URL. |
| README.md | Updates badges and usage examples to the new fit()-centric API and LoOP alias. |
| PyNomaly/loop.py | Implements the new estimator API shape (data args in fit()), deprecation warnings, refit reset behavior, and LoOP alias. |
| PyNomaly/exceptions.py | Extracts custom exceptions into a dedicated module. |
| PyNomaly/_validation.py | Adds validation mixin + accepts decorator used by the estimator. |
| PyNomaly/_utils.py | Extracts progress bar helper into a utility module. |
| PyNomaly/_pipeline.py | Extracts the LoOP scoring pipeline computations into a mixin. |
| PyNomaly/_distance.py | Extracts distance/neighbor computation into a mixin (vectorized + Numba paths). |
| PyNomaly/init.py | Re-exports LoOP and __version__ (plus exceptions) as public API. |
| docs/user-guide.md | Updates documentation code snippets to pass data to fit(). |
| docs/getting-started.md | Updates quickstart snippets to use LoOP and fit(data). |
| docs/changelog.md | Adds 1.0.0 changelog entry describing the API change and additions. |
| changelog.md | Mirrors the 1.0.0 changelog entry at repo root. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Clear self.data when switching to distance_matrix mode during re-fit, preventing _validate_inputs() from seeing conflicting inputs. - Accept DataFrame and np.ndarray for cluster_labels in the @accepts decorator to avoid spurious type warnings on the deprecated path. Co-authored-by: Cursor <cursoragent@cursor.com>
SaritNike
left a comment
There was a problem hiding this comment.
There are many compatibility issues with scikit-learn. Most of them are easy to fix and scikit learn provides helper functions to check the compatibility first. Instead of listing them all here, i will add a commit that includes this check in the unit tests.
| class MissingValuesError(ValidationError): | ||
| """Raised when data contains missing values.""" | ||
| pass | ||
| class LocalOutlierProbability(ValidationMixin, DistanceMixin, PipelineMixin): |
There was a problem hiding this comment.
Scikit learn requires following a certain inheritance structure (e.g. BaseEstimator) etc.
As it stands the code is currently not compatible with scikit-learn API
Summary
data,distance_matrix,neighbor_matrix,cluster_labels) from__init__()tofit(), aligning with scikit-learn estimator conventions. Configuration parameters (extent,n_neighbors,use_numba,n_jobs,progress_bar) remain in the constructor.__init__()still works but emits aFutureWarning.LoOPclass alias and exports__version__from the package.fit()multiple times with different data).Closes #5, Closes #7
Test plan
FutureWarning), numerical equivalence, re-fitting,fit()override precedence, andLoOPaliasMade with Cursor