fastFM
fastFM copied to clipboard
Add fit_predict to FactorizationMachine
Although required (and now overridden) by mcmc, having this method always present makes it easier to test multiple solvers with minimal code changes
I don't like to change the API except for a very good reason, especial if this moves us further away for the sklearn API. I'm not sure if easier to test qualifies as a strong enough reason. What do you think? Can you give an example where having fit_predict for sgd is better then fit(X,y).predict(X_test)?
I am not sure I would say this is changing the API but rather making it consistent. Once fit_predict has been added to the API for MCMC I think it is confusing to not have it available for SGD and ALS which can support it just as easily. This is in line with Python's duck typing which discourages needing to test for type before calling a method for similar objects. This additionally seems consistent with sklearn which has fit(), transform() and fit_transform() for most classes (even if fit_transform() is not needed or provides any advantage). If sklearn compatibility is a goal perhaps their terminology would be better?
I just revisit this PR once again but still I can't make up my mind if replacing fit_predict by fit_transform is the right way to go. It improves consistency (I would be fine with adding it to every model) and compatibility with sklearn. However the semantic is a bit confusing, that the reason why I went with fit_predict in the first place.