Skip to content

feat: adopt scikit-learn API conventions for 1.0.0#91

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

feat: adopt scikit-learn API conventions for 1.0.0#91
vc1492a wants to merge 8 commits into
devfrom
feat/sklearn-api-convention

Conversation

@vc1492a

@vc1492a vc1492a commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • Moves 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.
  • Adds backward compatibility: passing data params to __init__() still works but emits a FutureWarning.
  • Adds LoOP class alias and exports __version__ from the package.
  • Supports re-fitting (calling fit() multiple times with different data).
  • Bumps version to 1.0.0.

Closes #5, Closes #7

Test plan

  • All 40 existing tests updated to new API style and passing
  • New tests for backward compat (FutureWarning), numerical equivalence, re-fitting, fit() override precedence, and LoOP alias
  • CI passes across Python 3.8–3.14

Made with Cursor

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 Outdated
Comment thread PyNomaly/_validation.py Outdated
- 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
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

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