[MRG+2] MAINT Removing more deprecated stuff for v0.18 by raghavrv · Pull Request #5528 · scikit-learn/scikit-learn

raghavrv · 2015-10-22T12:29:21Z

Fully fixes #5434

fixed_vocabulary_
Remove support for precompute='auto' in ElasticNet
Remove support for loss='l2' in l1_min_c
Make all the old/new tests pass

raghavrv · 2016-04-13T19:38:32Z

Please review @amueller @ogrisel @MechCoder @TomDLT

TomDLT · 2016-04-13T19:50:13Z

sklearn/isotonic.py


-    @property
-    @deprecated("Attribute ``X_`` is deprecated in version 0.18 and will be"
-                " removed in version 0.20.")


not this? (0.20?)

Woooops. That is a bad blunder.

raghavrv · 2016-04-20T12:01:43Z

Could someone confirm if the deletions/movements in the metrics' tests are correct? @larsmans @jnothman @agramfort ?

raghavrv · 2016-04-20T14:10:30Z

@lesteve would you be able to take a look?

lesteve · 2016-04-20T14:42:37Z

sklearn/linear_model/coordinate_descent.py

-                          "slower even when n_samples > n_features. Hence "
-                          "it will be removed in 0.18.",
-                          DeprecationWarning, stacklevel=2)
+            raise ValueError("Precompute='auto' is not supported for "


precompute='auto' is still mentioned in the ElasticNet docstring above. You can probably remove it.

Indeed the docstring of the class needs to be updated.

I think the error message should just be the following:

if (isinstance(self.precompute, six.string_types): raise TypeError('Expected precompute=True or precompute=False, got precompute=%r' % precompute)

raghavrv · 2016-04-21T11:12:52Z

@ogrisel @TomDLT @lesteve Have addressed all the comments. One final look at the metrics/tests/* please?

lesteve · 2016-04-21T14:12:44Z

sklearn/linear_model/coordinate_descent.py

-                          DeprecationWarning, stacklevel=2)
+        if isinstance(self.precompute, six.string_types):
+            raise ValueError('precompute should be one of True, False or'
+                             ' array-like. Got %s' % self.precompute)


%r is slightly preferrable in this case because it will quote strings.

tguillemot · 2016-04-22T12:15:19Z

sklearn/metrics/tests/test_common.py

    "jaccard_similarity_score", "unnormalized_jaccard_similarity_score",
    "zero_one_loss", "unnormalized_zero_one_loss",

-    "precision_score", "recall_score", "f1_score", "f2_score", "f0.5_score",


Sorry for the misunderstanding but why have you removed this line ?

The precision_score is initiated with the default 'binary' averaging. Previously this was converted to 'weighted' for non-binary targets. Now that we are removing this behavior, this metric is not compatible with multilabel data.

So I'm removing it from this list to prevent it from getting tested with multilabel data.

raghavrv · 2016-04-22T12:16:11Z

I also have to edit this - http://scikit-learn.org/dev/modules/model_evaluation.html#from-binary-to-multiclass-and-multilabel

raghavrv · 2016-04-22T12:33:57Z

@TomDLT I'm retaining your +1 anyway ;)

tguillemot · 2016-04-22T13:02:12Z

LGTM too.

jnothman · 2016-04-30T09:36:18Z

sklearn/metrics/classification.py

+            raise ValueError('The average is chosen as binary, but the '
+                             'pos_label parameter is None')

    if y_type == 'binary' and pos_label is not None and average is not None:


I think what you want here is simply if y_type == 'binary' and average == 'binary' (i.e an else to the above ValueError case)

jnothman · 2016-08-03T10:39:17Z

@raghavrv, can we please split the non-P/R/F stuff out of this PR, so that it can be merged without discussion, then fix the P/R/F stuff with focus?

raghavrv · 2016-08-03T11:18:07Z

Done! review please ;)

jnothman · 2016-08-03T11:57:33Z

sklearn/linear_model/tests/test_coordinate_descent.py

+
+    # Precompute = 'auto' is not supported for ElasticNet
+    assert_raises_regex(ValueError, ".*should be.*True.*False.*array-like.*"
+                        "Got 'auto'", ElasticNet(precompute='auto').fit, X, y)


Why not in the iterated list above?

The error messages differ a bit. The previous case it should say that 'auto' is one of the valid options, in the current case it is not accepted anymore...

jnothman · 2016-08-03T12:00:17Z

otherwise, LGTM

jnothman · 2016-08-03T12:16:28Z

Thanks @raghavrv. Valuable work towards a release.

raghavrv · 2016-08-03T12:17:24Z

Thanks for the review and merge!

* MNT Removing more deprecated stuff for v0.18 * MNT+TST Remove support for loss='l2' * MNT Remove support for precompute='auto' for ElasticNet * FIX/TST precompute=auto should raise a generic message

raghavrv force-pushed the more_deprecated_stuff branch from c5f857a to a305c50 Compare October 23, 2015 13:06

raghavrv mentioned this pull request Nov 10, 2015

Remove deprecated stuff that will no longer be supported by 0.18 #5434

Closed

20 tasks

raghavrv force-pushed the more_deprecated_stuff branch from a305c50 to f6bc5ca Compare November 12, 2015 14:26

raghavrv force-pushed the more_deprecated_stuff branch from f6bc5ca to c8ccad9 Compare April 12, 2016 14:07

raghavrv changed the title ~~[WIP] MAINT Removing more deprecated stuff for v0.18~~ [MRG] MAINT Removing more deprecated stuff for v0.18 Apr 13, 2016

raghavrv force-pushed the more_deprecated_stuff branch 3 times, most recently from f95df96 to 656f1d3 Compare April 13, 2016 19:35

TomDLT reviewed Apr 13, 2016
View reviewed changes

raghavrv force-pushed the more_deprecated_stuff branch 2 times, most recently from 1882cee to 7474eff Compare April 14, 2016 08:55

TomDLT changed the title ~~[MRG] MAINT Removing more deprecated stuff for v0.18~~ [MRG+1] MAINT Removing more deprecated stuff for v0.18 Apr 14, 2016

raghavrv and others added 3 commits April 20, 2016 13:58

MNT Removing more deprecated stuff for v0.18

06a5c8f

MNT+TST Remove support for loss='l2'

8af35a2

MNT Remove support for precompute='auto' for ElasticNet

778cd7d

raghavrv force-pushed the more_deprecated_stuff branch from 7474eff to ee6f4fa Compare April 20, 2016 11:59

lesteve reviewed Apr 20, 2016
View reviewed changes

lesteve reviewed Apr 21, 2016
View reviewed changes

raghavrv force-pushed the more_deprecated_stuff branch 3 times, most recently from cecd6e3 to c02d004 Compare April 21, 2016 18:35

tguillemot reviewed Apr 22, 2016
View reviewed changes

jnothman reviewed Apr 30, 2016
View reviewed changes

jnothman added this to the 0.18 milestone Aug 3, 2016

FIX/TST precompute=auto should raise a generic message

2ece9bd

raghavrv force-pushed the more_deprecated_stuff branch from c02d004 to 2ece9bd Compare August 3, 2016 11:18

raghavrv mentioned this pull request Aug 3, 2016

[WIP] MNT Remove implicit 'binary' averaging with pos_labels #7134

Closed

jnothman reviewed Aug 3, 2016
View reviewed changes

jnothman changed the title ~~[MRG+1] MAINT Removing more deprecated stuff for v0.18~~ [MRG+2] MAINT Removing more deprecated stuff for v0.18 Aug 3, 2016

jnothman merged commit 2a9492b into scikit-learn:master Aug 3, 2016

raghavrv deleted the more_deprecated_stuff branch August 3, 2016 12:16

amueller mentioned this pull request Aug 7, 2019

LassoCV always sets precompute to False before fitting the chosen alpha value #11014

Closed

Uh oh!

Conversation

raghavrv commented Oct 22, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raghavrv commented Apr 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raghavrv commented Apr 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raghavrv commented Apr 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel Apr 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raghavrv commented Apr 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tguillemot Apr 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raghavrv commented Apr 22, 2016

Uh oh!

raghavrv commented Apr 22, 2016

Uh oh!

tguillemot commented Apr 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Aug 3, 2016

Uh oh!

raghavrv commented Aug 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Aug 3, 2016

Uh oh!

jnothman commented Aug 3, 2016

Uh oh!

raghavrv commented Aug 3, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

raghavrv commented Oct 22, 2015 •

edited

Loading

raghavrv commented Apr 20, 2016 •

edited

Loading

ogrisel Apr 20, 2016 •

edited

Loading

tguillemot Apr 22, 2016 •

edited

Loading