Evalml: Pipelines: standard error when calling transform/predict before fit

[jeremyliweishih]

In #969 (issue #851) we added an universal error for predict/transform before fitting for components using a metaclass to wrap predict/transform to check if the component has been fitted. We should add the same treatment for pipelines so that anything that inherits from PipelineBase receives the same validation.

[dsherry]

Yep. My thought is that we should generalize the metaclass pattern @jeremyliweishih added for components, and then use it on both components and pipelines.

[angela97lin]

JW: Do we not get this for "free" since our pipelines are made from components? When we call pipeline.predict, will it not boil down to the component's universal error?

[dsherry]

@angela97lin you're right! See screenshot below.

The question is, are we ok with the behavior below, or do we want the stack trace to be more direct? Currently, the stack trace comes from the first component. Ideally, it would come from the pipeline itself.

The quick way to make the stack trace more clear would be to except ComponentNotYetFittedError in the pipeline methods and then throw PipelineNotYetFittedError. Another way would be to recycle the metaclass/decorator strategy we took for components and apply it to the pipelines.

[angela97lin]

@dsherry Thanks for following up on this! I'll take on this issue, and try to address it via catching and throwing a PipelineNotYetFittedError error first, since I think there's no need to over-complicate if that suffices. If not, I'll try the metaclass approach :)

[angela97lin]

Hm yeah, upon actually implementing, it was more clear to me that the metaclass approach may be better since we won't have to stick a similar error in each of the methods we care about, so going to take that route instead.

[dsherry]

@angela97lin tl;dr: ok, sounds good!

We went with the metaclass strategy for checking this stuff in the components because our components are intended to be extended with custom fit, transform and predict methods, and the only sure way to get our validation code into those definitions was to inject them at class definition-time via the metaclass.

We're now in a place where our pipeline classes are intended as a template to wrap a graph of components, rather than directly holding custom implementations of fit/predict, although that is still supported and should remain supported for the time being. I think that's great. An implication of this is the metaclass strategy is not necessary to satisfy the goal of this issue, raising a clear error message when pipeline predict is called before fit.

However, implementing a pipeline metaclass now is an investment which would be nice to make. We have at least a couple issues outstanding tracking class definition-time validation of various fields on both pipelines and components. And a metaclass is almost certainly the right choice to get that done. So, full speed ahead! 🏎️😁