felixriese/susi

wrong optional type hint?

thunderbug1 opened this issue · 4 comments

Here it says that y is Optional which makes sense since for an unsupervised SOM it isn't necessary, however the check_estimation_input function does not allow y to be None.
Am I missing something?

We are inheriting from SOMEstimator which inherits from SOMClustering. In SOMClustering, the fit() function ignores y, in SOMClassifier, it does not. This solution is, therefore, a trade-off between inheritance compromises and complying with the scikit-learn estimator guidelines.

Oh ok I see, well thank you for the very fast reply.
Do you think it would make sense to change the fit method of SOMClassifier to check if it is setup for the unsupervised case and to ignore y as well in that case? I'm not sure if the guidelines say if it must not be ignored or not but then it would be consistent again.

I guess a simple workaround for it is to fill y with dummy values of correct length since the values should be ignored anyway.

The SOMClassifier is always supervised or semi-supervised. y is not ignored here.

If you want to use an unsupervised SOM, please use SOMClustering. y is ignored here.

Oh yes of course, sorry my bad, I had a thinking error.
Thanks for helping me out :)