Skip to content

Conversation

@Gnpd
Copy link
Owner

@Gnpd Gnpd commented Nov 1, 2025

This pull request introduces support for custom scikit-learn-compatible estimators in the SklearnSerializer, refactors the internal organization of sklearn-related serializers, and adds comprehensive tests for the new custom estimator logic. The changes improve extensibility, maintainability, and test coverage, allowing users to serialize and deserialize third-party or custom estimators alongside built-in ones.

Support for custom estimators in serialization:

  • Added a new mechanism to allow users to provide custom or third-party estimators to SklearnSerializer via a new custom_estimators parameter. These are merged into the serializer's internal registry for the instance, enabling seamless serialization/deserialization of external classes.
  • Implemented helper functions is_valid_estimator, normalize_estimators, and load_custom_estimators in openmodels/serializers/sklearn/_custom_estimator.py to validate, normalize, and load custom estimators.
  • Updated all usages of the estimator registry in SklearnSerializer to use the instance-specific registry (self._all_estimators) instead of the global one, ensuring custom estimators are recognized during (de)serialization.

Refactoring and reorganization:

  • Moved sklearn_serializer.py into a dedicated sklearn subpackage and updated all imports accordingly, improving codebase organization and clarity.

Solves #41

  • Updated documentation strings in SklearnSerializer to clarify the new extensibility features and provide developer guidance for third-party package maintainers.

Testing improvements:

  • Added a new test suite in test/test_custom_estimator.py covering all branches and edge cases of the custom estimator loading logic, including validation, normalization, error handling, and conflict resolution.

Other updates:

  • Minor docstring and version reporting improvements in SklearnSerializer.

Protocol and interface changes:

  • Updated the ModelSerializer protocol to allow arbitrary constructor arguments, supporting the new extensibility pattern.

@Gnpd Gnpd changed the title Feat/sklearn friends 41-Feat: sklearn friends Nov 1, 2025
@Gnpd Gnpd requested a review from Copilot November 1, 2025 15:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the sklearn serializer into a subdirectory structure and adds support for custom estimators. The main changes include reorganizing the codebase to use openmodels.serializers.sklearn.sklearn_serializer instead of the flat openmodels.serializers.sklearn_serializer, implementing a new custom estimator loading mechanism, and updating the sklearn version compatibility.

  • Refactored sklearn serializer into a subdirectory structure (sklearn/)
  • Added support for custom estimators via a new _custom_estimator module and constructor parameter
  • Updated tested sklearn version from "1.7.1" to "1.7.2"

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
openmodels/serializers/sklearn/sklearn_serializer.py Added __init__ method with custom_estimators parameter, updated documentation, changed from module-level ALL_ESTIMATORS to instance-level _all_estimators, updated TESTED_VERSIONS and producer_version logic
openmodels/serializers/sklearn/_custom_estimator.py New module implementing custom estimator validation, normalization, and loading logic
test/test_custom_estimator.py New comprehensive test suite for the _custom_estimator module
openmodels/serializers/init.py Updated import path from flat to nested structure
openmodels/protocols.py Added __init__ method to ModelSerializer protocol to allow serializers to accept constructor parameters
test/*.py Updated all import statements to use new nested module structure
openmodels/test_helpers.py Updated import statement to use new nested module structure
Comments suppressed due to low confidence (2)

openmodels/serializers/sklearn/sklearn_serializer.py:305

  • The comment references version '1.7.1' but TESTED_VERSIONS was updated to include '1.7.2'. This docstring should be updated to reflect the current tested versions or remove the specific version reference to avoid future inconsistency.
    openmodels/serializers/sklearn/sklearn_serializer.py:225
  • This class does not call ModelSerializer.init during initialization. (SklearnSerializer.init may be missing a call to a base class init)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Gnpd Gnpd linked an issue Nov 1, 2025 that may be closed by this pull request
@Gnpd Gnpd merged commit 488eb40 into main Nov 6, 2025
6 checks passed
@Gnpd Gnpd deleted the feat/sklearn-friends branch November 6, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for Scikit-learn-Compatible Third-Party Libraries

2 participants