WIP: extend Pipeline transform and inverse_transform behavior

[schwarty]

Pipeline transform and inverse_transform skip last step if it lacks corresponding methods.

The extension is useful to:

• extract the features before the last step of the pipeline for debugging purposes
• inverse_transform the coef_ attribute of a linear classifier for example

Design discussion here: Issue #2562

[coveralls]

Coverage remained the same when pulling d9260f0 on schwarty:pipeline_inverse_transform into 03926cc on scikit-learn:master.

[ogrisel]

Could you please edit the docstring of the inverse_transform method to explain that it works even if the last step does not support the inverse_transform method and give motivation for this feature as done for the forward case?

[ogrisel]

Looks good to me. @schwarty please add a doc/whats_new.rst entry for this.

[schwarty]

Done. @ogrisel and @GaelVaroquaux what do you think?

[coveralls]

Coverage remained the same when pulling 1906989 on schwarty:pipeline_inverse_transform into 03926cc on scikit-learn:master.

[ogrisel]

Please add double back-ticks around code words like transform and inverse_transform.

[ogrisel]

Apart from the nitpick comment, LGTM. +1 for merge.

[GaelVaroquaux]

As I just told @schwarty orally, I am not in favor of merging this PR, although I had backed the idea during our first discussions. I don't like the automatic change of behavior depending on subtle properties of the final learner. For instance changing the code to use an LDA instead of a LogisticRegression would have really strange consequences with this PR.

[ogrisel]

Do you have an alternative API to propose? Maybe a new constructor param?

I don't see your point with LDA. How this PR would change the behavior in this case? It is my understanding that only cases that would have previously raised an exception are now supported.

[GaelVaroquaux]

I don't see your point with LDA? How this PR would change the behavior in this
case? It is my understanding that only cases that would have previously raised
an exception are now supported.

Yes: with a LogisticRegression it would have raised an error, but not
with LDA. Now both work, but do something different. The difference is
subtle and can easily fool someone.

[ogrisel]

Hum ok I see your point. Still I think @schwarty's use cases are very valid and are not addressed by the current implementation. Maybe @schwarty you should put this PR back to WIP and raise the point on the mailing list to brain storm alternative design ideas.

[GaelVaroquaux]

Still I think @schwarty's use cases are very
valid and are not addressed by the current implementation.

I agree.

Maybe @schwarty you should put this PR back to WIP and raise the point
on the mailing list to brain storm alternative design ideas.

He has opened an issue.

[jnothman]

I am not in favor of merging this PR, although I had backed the idea during our first discussions. I don't like the automatic change of behavior depending on subtle properties of the final learner.

I agree with @GaelVaroquaux. It's a case of implicit behaviour that could bite you. What I wouldn't mind (though perhaps it's overkill and will create API problems) is for Pipeline to support slicing. I also think it deserves a getter for the final estimator (.steps[-1][1]) just to increase readability:

p = Pipeline([('sel', SelectKBest()), ('clf', LogisticRegression())])
p[:-1].inverse_transform(p.final_estimator.coef_)

[jnothman]

Or, just as slicing a list with an integer returns an element and with a slice returns a list, you could have integer slicing return just the estimator:

p[:-1].inverse_transform(p[-1].coef_)

[GaelVaroquaux]

I think that there is a consensus against merging this PR. @schwarty : is it OK with you if I close it.

[amueller]

closing as @schwarty didn't object within the last 3 years

[GaelVaroquaux]

closing as @schwarty didn't object within the last 3 years

@schwarty defended is PhD and has disappeared in the industry, making more money than me. He is still available to drink beers, though.

[jnothman]

I suppose it's good when people making more money still drink beers with you. :)