Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API - make BaseDatafit and BasePenalty regular classes #205

Merged

Conversation

Badr-MOUFAD
Copy link
Collaborator

@Badr-MOUFAD Badr-MOUFAD commented Nov 7, 2023

Context of the PR

Currently, BaseDatafit and BasePenalty classes don't force their children to implement methods.
It is actually impossible to enforce that by deriving Base classes from abs.ABC since Numba Jitclass still doesn't support it.

Also, several datafits and penalties diverge from the design prescribed by Base classes as they implement other methods to comply with solvers.

This PR is meant to overcome this ambiguity.

Closes #202

Contributions of the PR

  • Remove @abstractclass decorator from BaseDatafit and BasePenalty
  • Remove unused methods in BaseDatafit and BasePenalty
  • Update "add custom datafit/penalty" tutorials to reflect the changes

Checks before merging PR

  • [ ] added documentation for any new feature
  • [ ] added unittests
  • edited the what's new

@Badr-MOUFAD Badr-MOUFAD requested a review from mathurinm November 7, 2023 13:16
doc/changes/0.4.rst Outdated Show resolved Hide resolved
doc/tutorials/add_datafit.rst Outdated Show resolved Hide resolved
@mathurinm
Copy link
Collaborator

Thanks @Badr-MOUFAD , merge when green!

@Badr-MOUFAD Badr-MOUFAD merged commit 1b699a3 into scikit-learn-contrib:main Nov 14, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API simplify BaseDatafit and BasePenalty
2 participants