[MRG+1] Housekeeping Deprecations for v0.19 by amueller · Pull Request #7927 · scikit-learn/scikit-learn

amueller · 2016-11-23T22:58:24Z

Early PR for CI.

amueller · 2016-11-23T23:22:12Z

This again convinces me that #7618 would be a good idea (as default behavior even). The SVC changes a default parameter and I have to change doctests all over the project. ?!

jnothman · 2016-11-24T00:49:37Z

You're right. We should make more changes to public interfaces to save your maintenance effort :P

amueller · 2016-11-28T21:56:23Z

@jnothman you consider __repr__ and interface?

Also I just really really dislike our current __repr__. It's so useless and produces so much noise.

jnothman · 2016-11-28T22:45:41Z

I was only joking here. I'm coming to agree with you that we should change the default __repr__, though I still think it has great value is in showing users what parameters they might want to set. Another thing that we could do to improve our __repr__ might be ordering parameters by their order in signature or documentation rather than alphabetic...

amueller · 2016-11-29T00:40:00Z

Should be good now.

raghavrv · 2016-12-02T17:27:33Z

Can we decide to release 0.19 by early April 2017? We have ~4 months...

amueller · 2016-12-02T23:36:07Z

Yeah we should release April 1st and announce that fit will no longer return self ;)

But sounds good for a target date. If we can make it depends obviously on a lot of factors (I'll be teaching, for example).

jnothman

Also, class_weight auto should be removed from sklearn/linear_model/stochastic_gradient.py:498

Some docstrings in ..versionadded/..versionchanged reference the old deprecated value. Should we remove the trace of the old functionality? This affects 'auto' class_weight, and scaler's *scale_* is recommended instead of deprecated *std_*

Otherwise I think this is good.

jnothman · 2016-12-03T13:42:04Z

doc/modules/classes.rst

 Recently deprecated
 ===================

-To be removed in 0.19


ProjectedGradientNMF was missing here.

Should we list them again in the changelog? Or is that overkill?

Not sure what you're proposing, but I assume it's overkill :P

I mean "should 0.19 list everything that was removed". It might be useful but maybe not necessarily part of this PR ;) Do I have your +1?

Don't worry about it. Yes, you have my +1. I can't be bothered going through the details a second time!

Who's the dodgy one now ;)

I'm relying on the second reviewer being less dodgy??

Look, I think I did a pretty good job of catching some omissions here...

jnothman · 2016-12-03T13:46:42Z

sklearn/discriminant_analysis.py

        y : array, shape (n_samples,)
            Target values.
        """
-        if store_covariance:


Investigating any leftovers from this, I've discovered that QDA has store_covariances unlike LDA's store_covariance :((

open issue?

jnothman · 2016-12-03T14:00:33Z

sklearn/utils/class_weight.py

        # uniform class weights
        weight = np.ones(classes.shape[0], dtype=np.float64, order='C')
-    elif class_weight in ['auto', 'balanced']:
+    elif class_weight == 'balanced':


There are still two uses of 'auto' in compute_sample_weight and 1 more in its documentation.

amueller · 2016-12-07T16:30:23Z

@raghavrv wanna review? hm.. who else? @lesteve ? ;)

jnothman · 2016-12-07T20:56:33Z

LGTM

tguillemot · 2016-12-08T10:40:54Z

sklearn/discriminant_analysis.py

@@ -425,20 +425,6 @@ def fit(self, X, y, store_covariance=None, tol=None):
        y : array, shape (n_samples,)


Just a niptick but don't you need to change l414 - l418 to say that store_covariance and tol have been removed as you've done here ?

We don't actually have a policy on that, do we? But yeah, would be better I guess.

tguillemot · 2016-12-08T11:08:15Z

sklearn/tree/tests/test_tree.py

-        if tree in SPARSE_TREES:
-            yield (check_sparse_input, tree, dataset, 2)
+    for tree_type, dataset in product(SPARSE_TREES, ["boston", "reg_small"]):
+        if tree_type in REG_TREES:


I don't understand why you change that ?

because tree overwrites the tree module that was imported. And I (and flake8) don't like overwriting file-level module imports with local variable names.

Good for me :)

tguillemot · 2016-12-08T11:12:54Z

Do we have to update what's new ?
Anyway LGTM

…analysis

amueller · 2016-12-08T18:00:31Z

done

tguillemot · 2016-12-09T09:22:19Z

It's ok for me :)
Thanks @amueller

amueller · 2016-12-09T17:43:43Z

thanks for the reviews :)

raghavrv · 2016-12-09T18:06:52Z

Good bye 1200 lines of code 🎉

* remove stuff to be removed 0.19 * more changes * remove classes from 0.19 whatsnew * remove _LearntSelectorMixin * remove ProjectedGradientNMF, load_lwf_* * minor fixes * remove more copy from logistic regression path * remove lda, qda from __init__.__all__ * remove pg solver in nmf from tests etc * remove class_weight="auto" from tests * doctest change for decision_function_shape="ovr" * remove transfrom from tree test, minor fixes to tree tests * some fixes in the tests * undo changes in functions which still allow 1d input... * also allow 1d in scale * more test fixes... * last test fixes in forest and tree * svm default value change doctest failures * pep8 * remove more class_weight="auto" stuff * minor cosmetics in docstrings deprecated / removed behavior. * say that store_covariance has been moved to __init__ in discriminant_analysis

amueller changed the title ~~[WIP] Housekeeping Deprecations 019~~ [MRG] Housekeeping Deprecations 019 Nov 29, 2016

amueller mentioned this pull request Nov 29, 2016

[MRG+1] Add multiplicative-update solver in NMF, with all beta-divergence #5295

Merged

raghavrv changed the title ~~[MRG] Housekeeping Deprecations 019~~ [MRG] Housekeeping Deprecations for v0.19 Dec 2, 2016

raghavrv added this to the 0.19 milestone Dec 2, 2016

jnothman reviewed Dec 3, 2016

View reviewed changes

jnothman changed the title ~~[MRG] Housekeeping Deprecations for v0.19~~ [MRG+1] Housekeeping Deprecations for v0.19 Dec 7, 2016

tguillemot reviewed Dec 8, 2016

View reviewed changes

amueller added 12 commits December 8, 2016 12:52

remove stuff to be removed 0.19

dfe3314

more changes

807dbba

remove classes from 0.19 whatsnew

af93fa8

remove _LearntSelectorMixin

1e02202

remove ProjectedGradientNMF, load_lwf_*

d96bc95

minor fixes

4b3b141

remove more copy from logistic regression path

0c578ca

remove lda, qda from __init__.__all__

aaba7fd

remove pg solver in nmf from tests etc

f494ec1

remove class_weight="auto" from tests

4112056

doctest change for decision_function_shape="ovr"

412a9d5

remove transfrom from tree test, minor fixes to tree tests

76f9768

amueller added 10 commits December 8, 2016 12:52

some fixes in the tests

79ff9c7

undo changes in functions which still allow 1d input...

b6333ca

also allow 1d in scale

bd66a72

more test fixes...

5416a7f

last test fixes in forest and tree

7fe07e3

svm default value change doctest failures

5ba8b48

pep8

3e29fa2

remove more class_weight="auto" stuff

36e26b4

minor cosmetics in docstrings deprecated / removed behavior.

06056bc

say that store_covariance has been moved to __init__ in discriminant_…

af49979

…analysis

amueller force-pushed the deprecations_019 branch from caed135 to af49979 Compare December 8, 2016 18:00

amueller mentioned this pull request Dec 9, 2016

[MRG] Deprecated the rng_ attribute of GaussianProcessRegressor #8015

Closed

amueller merged commit 5c4b1bb into scikit-learn:master Dec 9, 2016

amueller mentioned this pull request Dec 13, 2016

[MRG] fix test for new default of SVC decision function #8050

Closed

adamjstewart mentioned this pull request May 6, 2024

DOC update r2_score default in regression metrics tutorial #28958

Merged

		@@ -425,20 +425,6 @@ def fit(self, X, y, store_covariance=None, tol=None):
		y : array, shape (n_samples,)

Uh oh!

Conversation

amueller commented Nov 23, 2016

Uh oh!

amueller commented Nov 23, 2016

Uh oh!

jnothman commented Nov 24, 2016

Uh oh!

amueller commented Nov 28, 2016

Uh oh!

jnothman commented Nov 28, 2016

Uh oh!

amueller commented Nov 29, 2016

Uh oh!

raghavrv commented Dec 2, 2016

Uh oh!

amueller commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman left a comment

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

amueller commented Dec 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Dec 7, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tguillemot commented Dec 8, 2016

Uh oh!

amueller commented Dec 8, 2016

Uh oh!

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

Uh oh!

Uh oh!

amueller commented Dec 9, 2016

Uh oh!

raghavrv commented Dec 9, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

amueller commented Dec 2, 2016 •

edited

Loading

amueller commented Dec 7, 2016 •

edited

Loading

tguillemot commented Dec 9, 2016 •

edited

Loading