Skip to content

SGDRegressor is not inheriting from LinearModel #31315

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

Open
MarcBresson opened this issue May 6, 2025 · 2 comments
Open

SGDRegressor is not inheriting from LinearModel #31315

MarcBresson opened this issue May 6, 2025 · 2 comments
Labels
Bug Needs Triage Issue requires triage

Comments

@MarcBresson
Copy link
Contributor

Describe the bug

I wanted to rely on the base class LinearModel to identify linear models, but I found out that SGDRegressor (nor any of its sub classes) is not inheriting this class. However, SGDClassifier is (through LinearClassifierMixin).

Is there any reason for BaseSGDRegressor to not inherit LinearModel? Is it because it overloads all of LinearModel's methods?

Steps/Code to Reproduce

from sklearn.linear_model import SGDRegressor
from sklearn.linear_model._base import LinearModel

issubclass(SGDRegressor, LinearModel)

Expected Results

from sklearn.linear_model import SGDRegressor
from sklearn.linear_model._base import LinearModel

issubclass(SGDRegressor, LinearModel)

True

Actual Results

from sklearn.linear_model import SGDRegressor
from sklearn.linear_model._base import LinearModel

issubclass(SGDRegressor, LinearModel)

False

Versions

System:
    python: 3.10.11 (v3.10.11:7d4cc5aa85, Apr  4 2023, 19:05:19) [Clang 13.0.0 (clang-1300.0.29.30)]
executable: /usr/local/bin/python3.10
   machine: macOS-14.4.1-arm64-arm-64bit

Python dependencies:
      sklearn: 1.5.0
          pip: 24.2
   setuptools: 74.0.0
        numpy: 1.26.4
        scipy: 1.13.1
       Cython: 3.0.12
       pandas: 1.5.3
   matplotlib: 3.8.4
       joblib: 1.2.0
threadpoolctl: 3.5.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/numpy/.dylibs/libopenblas64_.0.dylib
        version: 0.3.23.dev
threading_layer: pthreads
   architecture: armv8

       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/scipy/.dylibs/libopenblas.0.dylib
        version: 0.3.27
threading_layer: pthreads
   architecture: neoversen1

       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/sklearn/.dylibs/libomp.dylib
        version: None
@MarcBresson MarcBresson added Bug Needs Triage Issue requires triage labels May 6, 2025
@ogrisel
Copy link
Member

ogrisel commented May 6, 2025

Is there any reason for BaseSGDRegressor to not inherit LinearModel? Is it because it overloads all of LinearModel's methods?

Yes, I believe it was historically written independently of other linear models. Maybe subclassing from the base class could allow for some code reuse, although I have not checked.

Note that the LinearModel is not meant to be part of the public API and was not intended for model discovery but only for code reuse/factorization. AFAIK scikit-learn does not expose any public API to inspect which models can be considered linear models.

@ogrisel
Copy link
Member

ogrisel commented May 6, 2025

If subclassing can help remove redundant code, then I am not opposed to a PR. But if subclassing does not help reduce code complexity, I see little benefit to doing so. Extra indirections introduced by inheritance make it harder to navigate the code base and (slightly) hurt code readability as a result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Needs Triage Issue requires triage
Projects
None yet
Development

No branches or pull requests

2 participants