Skip to content

feat: adopt the scikit-learn estimator API for 1.0.0 - #91

Open
vc1492a wants to merge 29 commits into
devfrom
feat/sklearn-api-convention
Open

vc1492a wants to merge 29 commits into
devfrom
feat/sklearn-api-convention

Conversation

@vc1492a

@vc1492a vc1492a commented Jun 16, 2026 •

Copy link
Copy Markdown
Owner

PyNomaly 1.0.0 makes LocalOutlierProbability a proper scikit-learn estimator. This PR carries the whole change: the original refactor plus #92 and #93 from @SaritNike, which finished the compliance work and added a public API contract test.

What changed for users

  • Data moves out of the constructor. LocalOutlierProbability(extent, n_neighbors, use_numba, n_jobs, progress_bar) takes configuration only; data, distance_matrix, neighbor_matrix, and cluster_labels go to fit(X, y=None, ...). Passing them to the constructor by keyword still works with a FutureWarning. Passing data positionally to the constructor no longer works, since the first positional parameter is now extent.
  • The class inherits BaseEstimator and OutlierMixin, so get_params/set_params, fit_predict, predict, decision_function, and score_samples follow the sklearn outlier conventions (decision_function = score_samples - offset_, inliers are >= 0). check_estimator(LoOP()) passes with one documented xfail, check_fit1d, because LoOP deliberately accepts 1-d input and reshapes it to (-1, 1). scikit-learn is an optional dependency (pip install PyNomaly[sklearn]); a minimal fallback keeps the package importable without it.
  • Fitted state lives on _-suffixed attributes (local_outlier_probabilities_, prob_distances_, distance_matrix_, n_neighbors_, ...). Read-only aliases without the underscore remain for the output attributes. Note that distance_matrix after fit() is the constructor input (usually None); the computed matrix is only on distance_matrix_.
  • Exceptions now live in PyNomaly.exceptions and are re-exported from PyNomaly and PyNomaly.loop, so existing imports keep working.
  • LoOP is an alias for LocalOutlierProbability, and __version__ is exported from the package.
  • fit() can be called repeatedly with different data; state is reset each time.
  • scipy sparse matrices are accepted by fit, predict, decision_function, and stream (densified internally; scipy is a soft dependency).
  • stream() returns a Python float instead of a NumPy array.
  • Python 3.8 support is dropped; python_requires='>=3.9', classifiers for 3.9 through 3.14.
  • CI runs on every branch and pull request, and the flake8 syntax/undefined-name check now fails the job.

Migration

Before (0.4.x):

m = LocalOutlierProbability(data, extent=2, n_neighbors=20, cluster_labels=labels).fit()
scores = m.local_outlier_probabilities

After (1.0.0):

m = LoOP(extent=2, n_neighbors=20).fit(data, cluster_labels=labels)
scores = m.local_outlier_probabilities_

Deliberately deferred to the follow-up warnings PR

  • Removing the deprecated constructor data keywords (they warn today).
  • fit() still warns and returns an unfitted model on invalid input instead of raising ValueError; kept for backward compatibility for now.
  • Documenting the non-underscore read aliases as deprecated, and eventually warning on access.
  • cluster_labels_u became private (_cluster_labels_u); it needs a changelog note if we consider it part of the public API.

Tests as of the compliance head 43ea6e4: 52 passed, 4 skipped locally; CI green on 3.9 through 3.14.

Thanks to @SaritNike for #92 (the compliance work and check_estimator integration) and #93 (the public API contract test).

Closes #7

vc1492a and others added 6 commits May 26, 2026 11:47
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>
@vc1492a vc1492a self-assigned this Jun 16, 2026
@vc1492a vc1492a added the enhancement New feature of request label Jun 16, 2026
@vc1492a
vc1492a requested a review from Copilot June 16, 2026 17:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) to fit() and add FutureWarning-based backward compatibility for passing them to __init__().
  • Refactor the implementation into mixins/modules (_validation, _distance, _pipeline, _utils) and add LoOP alias + __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.

Comment thread PyNomaly/loop.py Resolved
Comment thread PyNomaly/_validation.py Resolved
- 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
SaritNike self-requested a review June 26, 2026 11:01

@SaritNike SaritNike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread PyNomaly/loop.py Outdated
class MissingValuesError(ValidationError):
"""Raised when data contains missing values."""
pass
class LocalOutlierProbability(ValidationMixin, DistanceMixin, PipelineMixin):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

SaritNike and others added 16 commits June 26, 2026 13:25
- Update model state validation in stream() to raise Scikit-Learn's NotFittedError.
- Track n_features_in_ and validate feature consistency in predict() and decision_function().- Change generated variable names to include _ suffix and add property decorators to ensure backward compatibility
- Fix class inheritance MRO order by moving BaseEstimator to the end.
- Enhance _convert_to_array() to reject empty arrays, complex types, and invalid numeric inputs with appropriate exceptions as scikit-learn demands.
- Fortify _check_missing_values() to validate both NaN and infinite values, and update test assertions.
Add a _compat shim so scikit-learn remains an extras install, densify sparse inputs, keep init params immutable during fit, and finish the outlier API (score_samples, offset_, decision/predict consistency) so check_estimator passes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Accept scipy.sparse input by densifying at validation, fix stream() for sparse rows, raise a clear ImportError without scipy, and document the behavior plus a PyNomaly[sparse] extra.

Co-authored-by: Cursor <cursoragent@cursor.com>
Align the annotation and API docs with the actual scalar return value, and note the type change in the 1.0.0 changelog.

Co-authored-by: Cursor <cursoragent@cursor.com>
Restrict the deprecated positional distance_matrix fallback in fit() to calls with no raw data and a y wider than one column, so sklearn-style fit(X, y) with (n, 1) labels fits normally instead of returning an unfitted model. Adds a regression test.

Co-authored-by: Cursor <cursoragent@cursor.com>
Move data from positional constructor calls into fit(), fix the distance-matrix self-removal NameError in README/user-guide, and note the positional constructor break in the 1.0.0 changelog.

Co-authored-by: Cursor <cursoragent@cursor.com>
pandas is not a hard dependency, so bind the name only under
typing.TYPE_CHECKING for the string type hints instead of importing it
at runtime.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lity.

The exceptions moved to PyNomaly.exceptions in 1.0.0 and were no longer
importable from PyNomaly.loop. Import them back into loop, list them in
loop.__all__, add a test for the import path, and document the new home
in the changelog and API docs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e job.

Drop --exit-zero from the E9/F63/F7/F82 flake8 run so syntax errors and
undefined names fail the build. The style/complexity run stays advisory.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ed_failed_checks.

The keyword was added in scikit-learn 1.6, which needs Python 3.9+.
On Python 3.8 (scikit-learn 1.3.x) the test raised TypeError and failed
CI; it now skips with a clear reason there and runs everywhere else.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Remove 3.8 from the CI matrix, declare python_requires='>=3.9' with
3.9 through 3.14 classifiers in setup.py, update the supported-version
statements in the README and docs, and record the removal under 1.0.0
in the changelog.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
vc1492a and others added 5 commits September 17, 2026 00:57
Every remaining CI Python version (3.9+) resolves scikit-learn 1.6.1 or
newer, all of which support check_estimator(expected_failed_checks=...),
so the compliance test now always runs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Covers 1-d, (n, 1), and (n, 2) label arrays alongside raw data X,
asserting the raw-data path is used, no FutureWarning fires, and the
scores equal fit(X). Complements the (n, 1) test added in 9f74189 for
the fit(X, y) finding on PR #92.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Translate SaritNike's contract test from the 0.4.0 constructor to the
scikit-learn-style API: configuration-only constructor, data passed to
fit(X, y=None, ...), _-suffixed fitted attributes with their read-only
legacy aliases, the sklearn outlier methods, get_params() keys, the LoOP
alias, and the exception classes now defined in PyNomaly.exceptions and
re-exported from PyNomaly.loop. The structure and intent of the original
test are kept; cluster_labels_u is dropped from the expected attributes
because it became private (_cluster_labels_u) in 1.0.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@vc1492a vc1492a changed the title feat: adopt scikit-learn API conventions for 1.0.0 feat: adopt the scikit-learn estimator API for 1.0.0 Sep 22, 2026
@vc1492a
vc1492a dismissed SaritNike’s stale review September 22, 2026 18:01

Dismissing the June review: its findings were addressed in #92, now merged into this branch as a35b62e. Re-requesting review on the combined 1.0.0 change.

@vc1492a
vc1492a requested a review from SaritNike September 22, 2026 18:01

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature of request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants