[MRG+1] Deprecate fetch_mldata #11466

jnothman · 2018-07-10T10:29:23Z

I have changed the use of fetch_mldata in examples, but not yet in benchmarks under the assumption that those will be replaced with fetch_openml soon enough (even if after 0.20 release).

Waiting on the OpenML fetcher (#11419) to be completed.

jnothman · 2018-07-11T00:54:10Z

It seems there are bugs in scipy.io.arff.loadarff in our earliest supported dependencies. Maybe this will need to wait for the OpenML fetcher (#9908, #11419)

qinhanmin2014 · 2018-07-14T12:38:11Z

I'm fine with the deprecation, but do we have ways to make the examples more friendly? (e.g., make use of scikit-learn/examples-data) I'm a bit uncomfortable about things like from sklearn.externals.joblib import Memory and the long functions which download&preprocess the data.

amueller · 2018-07-14T15:54:30Z

all the fetchers can be removed when we add the openml loader, right? If not, we should upload those datasets to openml.

FollowKenny · 2018-07-16T09:25:37Z

@jnothman Hi, do you need help on this right now ?

jnothman · 2018-07-16T13:33:21Z

No, thanks @FollowKenny. The plan is to get fetch_openml merged before release, I hope, and fix the issues here through that.

jnothman · 2018-08-15T05:25:34Z

This isn't perfect, but it's mergeable after #11419

jnothman · 2018-08-15T05:29:23Z

We should also look into uploading Mauna Loa to openml

jnothman · 2018-08-15T05:57:13Z

I think if this goes green, I'll be bold and merge #11419 and create a long series of follow-up issues.

jnothman · 2018-08-15T07:24:30Z

This is now ready for review.

rth · 2018-08-15T07:30:55Z

doc/datasets/index.rst

-
-Downloading datasets from the mldata.org repository
---------------------------------------------------
-


Do you think it's better to remove this section altogether rather than say put a deprecation note at the begining?

rth · 2018-08-15T07:32:40Z

doc/modules/classes.rst

@@ -257,7 +257,6 @@ Loaders
   datasets.fetch_kddcup99
   datasets.fetch_lfw_pairs
   datasets.fetch_lfw_people
-   datasets.fetch_mldata


Same here, maybe still keep the doc even if it's deprecated? Who knows maybe the website will work again in september, and there is still code using this. Those users will no longer be able to find any documentation about this function in the stable docs... A doc with a large deprecation warning would avoid us some opened issues from confused users.

rth · 2018-08-15T07:33:41Z

examples/gaussian_process/plot_gpr_co2.py

-data = fetch_mldata('mauna-loa-atmospheric-co2').data
-X = data[:, [1]]
-y = data[:, 0]
+def load_mauna_loa_atmospheric_c02():


We should open an issue that this should be uploaded to openml, as you suggested?

rth · 2018-08-15T07:39:03Z

sklearn/datasets/tests/test_mldata.py

                      fetch_mldata, 'not_existing_name')
    finally:
        datasets.mldata.urlopen = _urlopen_ref


-def test_fetch_one_column(tmpdata):
+def test_fetch_one_column(tmpdata, recwarn):
+    warnings.simplefilter('ignore', DeprecationWarning)


Maybe rather use,

@pytest.mark.filterwarnings('ignore::DeprecationWarning'): def test_fetch_one_column(tmpdata): ...

if I got the syntax right. I'm never sure about the side effects explicitly setting warning filters could have in a pytest session..

I thought recwarn grabbed a filter context... But you're right that syntax is a little better.

It does, whether this isolates the filter context (and will continue doing so in the future) is another story. That's why using standard pytest mechanisms is probably safer (besides syntax considerations).

rth · 2018-08-15T07:39:13Z

sklearn/datasets/tests/test_mldata.py

@@ -82,7 +87,8 @@ def test_fetch_one_column(tmpdata):
        datasets.mldata.urlopen = _urlopen_ref


-def test_fetch_multiple_column(tmpdata):
+def test_fetch_multiple_column(tmpdata, recwarn):
+    warnings.simplefilter('ignore', DeprecationWarning)


Same comment as above

jnothman · 2018-08-15T07:42:25Z

doc/modules/classes.rst

@@ -1513,6 +1512,7 @@ To be removed in 0.22
   :template: deprecated_function.rst

   covariance.graph_lasso
+   datasets.fetch_mldata


This will generate the docs with the large deprecation warning.

jnothman · 2018-08-15T07:44:15Z

sklearn/datasets/tests/test_mldata.py

                      fetch_mldata, 'not_existing_name')
    finally:
        datasets.mldata.urlopen = _urlopen_ref


-def test_fetch_one_column(tmpdata):
+def test_fetch_one_column(tmpdata, recwarn):
+    warnings.simplefilter('ignore', DeprecationWarning)


I thought recwarn grabbed a filter context... But you're right that syntax is a little better.

amueller · 2018-08-16T20:58:00Z

are these test failures from master?

amueller · 2018-08-16T21:38:27Z

sklearn/datasets/tests/test_mldata.py

@@ -3,6 +3,7 @@
 import os
 import scipy as sp
 import shutil
+import warnings


not used (that's the flake8 error)

amueller · 2018-08-16T21:41:50Z

is that the failure that's fixed in #11831?

rth · 2018-08-17T14:40:26Z

mldata doctest is failing as far as I can see. We might want to skip it in pytest_collection_modifyitems under conftest.py ?

qinhanmin2014

LGTM, +1 to skip the test (maybe # doctest: +SKIP?)
Maybe we should include it in the rc

qinhanmin2014 · 2018-08-17T15:31:57Z

doc/whats_new/v0.20.rst

@@ -209,6 +209,10 @@ Support for Python 3.3 has been officially dropped.
  data points could be generated. :issue:`10045` by :user:`Christian Braune
  <christianbraune79>`.

+- |API| Deprecated :func:`sklearn.datasets.fetch_mldata` to be removed in
+  version 0.22. MLData.org is no longer operational. Until removal it will


mldata.org?

qinhanmin2014 · 2018-08-17T15:35:32Z

examples/linear_model/plot_sgd_early_stopping.py

    """Load MNIST, select two classes, shuffle and return only n_samples."""
-    mnist = fetch_mldata('MNIST original')
+    mnist = fetch_openml('mnist_784', version=1)


(alternative) Maybe we can add a link to openml in the examples (e.g., https://www.openml.org/d/554) so that users can get more information about the dataset.

rth · 2018-08-17T15:41:15Z

(maybe # doctest: +SKIP?)

Definitely, I'm spending too much time writing pytest fixtures ..

jnothman · 2018-08-18T09:45:18Z

I just deleted the example. If someone wants to remind me the right way to disable it...

qinhanmin2014

LGTM, I'm fine with removing the doctests. We should encourage users to turn to openml.
Appveyor failure irrelevant.

rth · 2018-08-18T16:27:19Z

Merging, thanks @jnothman !

jnothman changed the title ~~Deprecate fetch_mldata~~ [MRG] Deprecate fetch_mldata Jul 10, 2018

jnothman changed the title ~~[MRG] Deprecate fetch_mldata~~ [WIP] Deprecate fetch_mldata Jul 10, 2018

jnothman changed the title ~~[WIP] Deprecate fetch_mldata~~ [MRG] Deprecate fetch_mldata Jul 10, 2018

jnothman force-pushed the mldata branch from 5d3e768 to ce0d24a Compare July 10, 2018 23:28

jnothman changed the title ~~[MRG] Deprecate fetch_mldata~~ [WIP] Deprecate fetch_mldata Jul 11, 2018

jnothman added this to the 0.20 milestone Jul 11, 2018

amueller added the Blocker label Jul 16, 2018

janvanrijn mentioned this pull request Jul 25, 2018

[MRG] Openml data loader #11419

Merged

jnothman force-pushed the mldata branch from 32de9ff to df22dfa Compare August 15, 2018 05:24

jnothman changed the title ~~[WIP] Deprecate fetch_mldata~~ [MRG] Deprecate fetch_mldata Aug 15, 2018

jnothman changed the title ~~[MRG] Deprecate fetch_mldata~~ [WIP] Deprecate fetch_mldata Aug 15, 2018

API Deprecate fetch_mldata and update examples

60d08b9

jnothman force-pushed the mldata branch from c082675 to 60d08b9 Compare August 15, 2018 07:24

jnothman changed the title ~~[WIP] Deprecate fetch_mldata~~ [MRG] Deprecate fetch_mldata Aug 15, 2018

rth reviewed Aug 15, 2018

View reviewed changes

jnothman commented Aug 15, 2018

View reviewed changes

jnothman mentioned this pull request Aug 15, 2018

fetch_openml should support return_X_y #11814

Closed

Use pytest's filterwarnings

52491a1

panzhufeng mentioned this pull request Aug 16, 2018

[MRG+1] fetch_openml should support return_X_y #11840

Merged

amueller reviewed Aug 16, 2018

View reviewed changes

Rm unused import

ea8f31b

qinhanmin2014 reviewed Aug 17, 2018

View reviewed changes

jnothman added 4 commits August 18, 2018 19:40

Remove broken doctest

1b40d08

Merge branch 'mldata' of github.com:jnothman/scikit-learn into mldata

63c9509

Refer user to openml URL

ee3784f

DOC whatsnew tweak

e9a59c7

qinhanmin2014 approved these changes Aug 18, 2018

View reviewed changes

qinhanmin2014 changed the title ~~[MRG] Deprecate fetch_mldata~~ [MRG+1] Deprecate fetch_mldata Aug 18, 2018

rth merged commit fc56da5 into scikit-learn:master Aug 18, 2018

rth mentioned this pull request Aug 18, 2018

[MRG+1] FIX hotfix for joblib#741 #11837

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG+1] Deprecate fetch_mldata #11466

[MRG+1] Deprecate fetch_mldata #11466

jnothman commented Jul 10, 2018 •

edited

Loading

jnothman commented Jul 11, 2018

qinhanmin2014 commented Jul 14, 2018

amueller commented Jul 14, 2018

FollowKenny commented Jul 16, 2018

jnothman commented Jul 16, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

rth Aug 15, 2018

rth Aug 15, 2018

rth Aug 15, 2018

rth Aug 15, 2018 •

edited

Loading

jnothman Aug 15, 2018

rth Aug 15, 2018

rth Aug 15, 2018

jnothman Aug 15, 2018

jnothman Aug 15, 2018

amueller commented Aug 16, 2018

amueller Aug 16, 2018 •

edited

Loading

amueller commented Aug 16, 2018

rth commented Aug 17, 2018

qinhanmin2014 left a comment

qinhanmin2014 Aug 17, 2018

qinhanmin2014 Aug 17, 2018

rth commented Aug 17, 2018

jnothman commented Aug 18, 2018

qinhanmin2014 left a comment

rth commented Aug 18, 2018


		Downloading datasets from the mldata.org repository
		---------------------------------------------------

[MRG+1] Deprecate fetch_mldata #11466

[MRG+1] Deprecate fetch_mldata #11466

Conversation

jnothman commented Jul 10, 2018 • edited Loading

jnothman commented Jul 11, 2018

qinhanmin2014 commented Jul 14, 2018

amueller commented Jul 14, 2018

FollowKenny commented Jul 16, 2018

jnothman commented Jul 16, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

jnothman commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rth Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amueller commented Aug 16, 2018

amueller Aug 16, 2018 • edited Loading

Choose a reason for hiding this comment

amueller commented Aug 16, 2018

rth commented Aug 17, 2018

qinhanmin2014 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rth commented Aug 17, 2018

jnothman commented Aug 18, 2018

qinhanmin2014 left a comment

Choose a reason for hiding this comment

rth commented Aug 18, 2018

jnothman commented Jul 10, 2018 •

edited

Loading

rth Aug 15, 2018 •

edited

Loading

amueller Aug 16, 2018 •

edited

Loading