I am also thinking that it could be great to open a follow-up issue to address the remaining issue:

• validation on transform and predict (and friends) methods;
• include sample_weight;
• maybe modify check_X_y when y is required or not.

Thanks for the reviews !

On a side note, I find weird that we will soon make fail some third-party estimators without providing _validate_data as a developer tool

Just to clarify, the common check only raises a warning for now. Also, our plan is to decide whether we make _validate_data public before 0.23, so that should be a no-issue. I'll make sure to open the discussion about this once the PR addressing _validate_data in predict/transformed is merged (if it hasn't been decided before).

It looks good. We could merge I think @NicolasHug

Ups wrong PR sorry (too many tabs opened :))

Maybe let's wait for @jnothman's final review before merging.

I think we need to decide on https://github.com/scikit-learn/scikit-learn/pull/16112/files#r378281885 before merging?

During the meeting we decided to merge this PR and follow up with the introduction of a 'is_supervised' tag, and to raise a proper error message in validate_data() when y is None and the tag is True.

I gave it a try but I don't think it will completely work: some estimators like ElasticNetCV will validate y and X separetely, e.g. to fail early if y isn't of the right shape. More concretely, they will first call y = check_array(y, ...), and then X = _validate_data(X, ...), but they don't pass y to _validate_data(). So now, with the addition of the tag and the addition of the check, _validate_data() fails with "This estimator is supervised but y is None". That might happen for third-party estimators as well.

Do you want to fix conflicts so we can merge?
I don't like the bad error message but I think we decided in the meeting that we can merge now and fix that before the release. I think we should certainly fix it before the release, but the amount of merge conflicts suggests to me that it might be good to merge this now.

As much as I want this to be merged, we don't have a viable fix at the moment. It seems like the tag won't work out (see message above). Do you think we should still merge?

I feel pretty confident we can find a solution, though.

So either we say relying on a tag is bad because it doesn't allow us to be flexible, and we pass it explicitly every time, or we refactor elasticnet to pass y.

There's a bunch of possible solutions, I think:

• add a require_y argument everywhere
• refactor ElasticNet and use the tag
• use the tag but allow overwriting by an explicit require_y argument
• always return y and raise an error outside of _validate_data

OK, i'll fix the conflicts and merge when green

thanks everyone for the reviews on this

It's green! merging. Thanks again for the reviews