I didn't add any warnings for this change of behavior because fetch_openml is an experimental feature: https://scikit-learn.org/stable/modules/generated/sklearn.datasets.fetch_openml.html

Let me know if you guys think we should add a warning.

@glemaitre thanks for review, will follow the guidelines in the doc.

Sorry I am not familiar with sklearn development policies. Will read the docs more.

Don't worry. That's why I give pointer :)

hi @glemaitre I added the future warning and corresponding test, could you review?

@fujiaxiang there was a conflict in the whats_new I took the liberty of fixing it, hopefully you don't mind too much.

@glemaitre thanks for review, have updated the test and docstring as requested.

@lesteve Not at all, thanks! I also took the liberty to reorder the whatsnew entries a bit by their labels (i.e Enhancement comes before API). Could you review?

I think we should either:

1. return a dataframe for any dataset that fails with as_frame=False (e.g. non-numeric datasets); or
2. given the experimental label, simply adopt as_frame='auto' but raise a ChangedBehaviourWarning in the case that as_frame=False would have returned an array, but now will return a dataframe (e.g. all-numeric data)

Hi @jnothman, thanks for the two suggestions.

I am not particularly fond of Option 1. As per discussions in #14888 and #17396, it causes confusion to users if the return type depends on the data content itself, and potentially causes unwanted exceptions. It also defeats the purpose of having 'auto' being one of the option for as_frame.

I think Option 2 is ok. Although, as @glemaitre and @lesteve pointed out, we have already compensated for the experimental label by shortening the deprecation cycle by 1 version. Do you think this is still too conservative? Another issue we face now is that ChangedBehaviourWarning itself is deprecated in 0.24. If we are to go with Option 2, is there an alternative to it (perhaps UserWarning)? I didn't find such info in the deprecation message.

given the experimental label, simply adopt as_frame='auto' but raise a ChangedBehaviourWarning in the case that as_frame=False would have returned an array, but now will return a dataframe (e.g. all-numeric data)

I am + 1 with option 2. I would be in favor to keep ChangedBehaviour for these specific case.

@glemaitre @jnothman I have updated the code to simply adopt new default value as_frame='auto' in 0.24 and raises ChangedBehaviorWarning in cases where it used to return array but now returns DataFrame.

For now I have to include a filter to ignore the deprecation warning of ChangedBehaviorWarning in the test.

@adrinjalali also mentioned in #17804 that perhaps we shouldn't even raise a warning for experimental features.
This seems to be inline with other similar projects pandas and tensorflow, which follows or loosely follows Semantic Verisoning 2.0.
I couldn't find any scikit-learn documentation that explicitly discussed version policy for experimental features, so I took a look at similar projects tensorflow and pandas.

In Tensorflow version policy (https://www.tensorflow.org/guide/versions)

Some parts of TensorFlow can change in backward incompatible ways at any point. These include:

• Experimental APIs:

In Tensorflow documentation (https://www.tensorflow.org/probability/api_docs/python/tfp/experimental)

tfp.experimental has no API stability guarantee. The public footprint of tfp.experimental code may change without notice or warning.

Also in Tensorflow (https://www.tensorflow.org/api_docs/python/tf/autograph/experimental/Feature)

These conversion options are experimental. They are subject to change without notice and offer no guarantees.

In Pandas version policy (https://pandas.pydata.org/docs/development/policies.html)

Pandas may change the behavior of experimental features at any time.

Also in pandas (https://pandas.pydata.org/pandas-docs/version/1.0.0/whatsnew/v1.0.0.html)

Experimental: the behaviour of pd.NA can still change without warning.

StringDtype is currently considered experimental. The implementation and parts of the API may change without warning.

Both allows experimental features to change any time, and may do so without warning.

So I think it is ok to make this change without any warning. It is a matter of how you guys (sklearn core development team) want to set the policy. I myself kind of prefer the flexibility to change experimental features without warning, as this allows developers to quickly test new features among receive community feedback. Another small downside of giving ChangedBahaviorWarning is when a behavior is changed a second time while the feature is still experimental. It may confuse users what's actually going on.

Perhaps you guys can have a discussion and decide what kind of policy/guideline you want to set?

Yes, I would be very much happier if we treat the experimental features as experimental and change them when needed without a warning. The warnings unnecessarily clutter user's space and prevent us from raising warnings in places where we actually need to raise a warning (bad default param and intercept scaling just two examples).

@fujiaxiang I know you really mean no harm, but it would be much nicer if you could avoid using "you guys" (https://youguys.club/). Thanks :)

@adrinjalali @glemaitre @jnothman @lesteve I removed the warning message. Could you review?

I know you really mean no harm, but it would be much nicer if you could avoid using "you guys"

@adrinjalali Sure, will avoid using that phrase.

ping

@adrinjalali @glemaitre @jnothman @lesteve Anyone can help review?

Thanks @fujiaxiang