SGDRegressor is not inheriting from LinearModel #31315
Comments
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 |
|
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. |
|
I looked a bit at the code and I think that we can clean-up something. In addition of the inheritance, I think we should look at the code of scikit-learn/sklearn/linear_model/_base.py Lines 322 to 324 in 81bb708 I think there is room to improve the clarity of those classes and it should remove some redundant code, notably when it comes to the |
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: NoneThe text was updated successfully, but these errors were encountered: