pymc-marketing

pymc-marketing copied to clipboard

What should be returned when fit is called?

fitted_model = Model(data).fit() is a common convention amongst those familiar with the sklearn API, but won't work unless self is returned. Currently this method returns either None or model.fit_result for all models. The latter could confuse end users if they try to call predictive methods. Either way, all models should be consistent in this regard.

Related to https://github.com/pymc-labs/pymc-marketing/issues/318 and https://github.com/pymc-labs/pymc-marketing/issues/319, instead of calling build_model() within fit(), it'd make more sense to do it within the subclass __init__. This would allow support for external samplers and also ensure repr works properly.

I think fit should return the fit results, regardless of what sklearn does. I have seen code examples use it already, which means it would also break existing code.

Calling build_model in the CLV cases makes sense, but if the reason is only the __repr__ we can also change the __repr__ so it's not based on a built model. No strong preference here.