Not sure if the functions ensure_consistent_length is in the right place there, or if it should go in some utils module.

Thanks for the feedback! I tried to incorporate the requested changes.

I renamed the function, let me know what you think. If you still don't like it I can inline the code.

Please add an entry to the change log at doc/whats_new/v0.21.rst. Please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:

@albertcthomas Thanks for the review! in the model evaluation docs in the dummy-estimator section http://scikit-learn.org/stable/modules/model_evaluation.html#dummy-estimators it says
"Note that with all these strategies, the predict method completely ignores the input data!".
So there is something in the docs. One could of course add tests for every dummy estimator and every strategy, so the estimator ones fitted produces the same prediction for different X.

The definition of a dummy estimator as predicting the label based only on the label distribution and independent of the features makes a lot of sense to me, as this is so to say the best you can get when your features have absolutely no predictive power.

I could add some test if you think it makes sense.

Ok, I added some tests for checking the independence of X. If someone would implement a strategy that does not operate independently of X, they would of course have to add their strategy to the test to see it fail. But I guess that should make it a bit more obvious that this is not desired.

The definition of a dummy estimator as predicting the label based only on the label distribution and independent of the features makes a lot of sense to me, as this is so to say the best you can get when your features have absolutely no predictive power.

I agree. My point is just that if a contributor or a user adds a dummy strategy that depends on X then returning the score independently of X would be wrong and this could fail silently.

I agree. My point is just that if a contributor or a user adds a dummy strategy that depends on X then returning the score independently of X would be wrong and this could fail silently.

One possibility would be to go from

X = np.zeros(shape=(len(y), 1))

back to

X = [[None]] * len(y)

so when some computation on the made up X is attempted, the operation would fail, if predict actually uses the values from X. Additionally, one could add even more explicit documentation. What do you think?

To be completely safe, another possibility would be to add a check in clf.score such as

if X is None and strategy not in {current strategies}:
  raise ValueError('informative message')

To be completely safe, another possibility would be to add a check in clf.score such as

if X is None and strategy not in {current strategies}:
    raise ValueError('informative message')

Not really sure about that. A similar check is already performed in clf.fit. So if someone would implement a new strategy, they would have to add their strategy to this check anyway. Therefore, they would also add it to to this check if we put it in the code.

Adding the a second strategy check to clf.score would of course give the possibility to have some strategies that work independent of X and some that work not independent of X. However, I think it would make more sense to clearly define a DummyEstimator as an estimator that ignores X and not even introduce the possibility to implement something else.

Let's sneak this into 0.20.
Please add an entry to the change log at doc/whats_new/v0.20.rst. Like the other entries there, please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:

@jnothman I moved the entry to doc/whats_new/v0.20.rst

Thanks @JarnoRFB!

Thanks for guiding me through this. Very happy to contribute!