Inherit from ``sklearn.base``
jakevdp opened this issue · 3 comments
To make this maximally compatible with scikit-learn, we should have the estimators inherit from sklearn.base, and then run sklearn's check_estimator on each as part of the test suite.
The only thing to keep in mind is that sklearn estimators can't do anything in __init__ except store the arguments by name (this is to enable cloning the estimator for grid search & parallelization). It seems like we're already in good shape as far as that requirement.
To Do: add check_estimator to embedding testing. Make sure it works.
I can take a look at that now
check_estimator is failing for a number of reasons: mainly our decision not to have a default value for the radius, and also with some issues in scikit-learn itself (e.g. scikit-learn/scikit-learn#6510). We'll leave this as a "will not fix" for the time being.