8000 [MRG] MNT Remove code deprecated in 0.18 by massich · Pull Request #10094 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG] MNT Remove code deprecated in 0.18 #10094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 14 commits into from
Closed

[MRG] MNT Remove code deprecated in 0.18 #10094

wants to merge 14 commits into from

Conversation

massich
Copy link
Contributor
@massich massich commented Nov 9, 2017 •
edited
Loading

Remove code tagged to be removed in v.0.20. This PR takes over #9570 (hopefully I've migratted everything).

Things to remove:

  • Classes (reported here)

    • cross_validation.KFold
    • cross_validation.LabelKFold
    • cross_validation.LeaveOneLabelOut
    • cross_validation.LeaveOneOut
    • cross_validation.LeavePOut
    • cross_validation.LeavePLabelOut
    • cross_validation.LabelShuffleSplit
    • cross_validation.ShuffleSplit
    • cross_validation.StratifiedKFold
    • cross_validation.StratifiedShuffleSplit
    • cross_validation.PredefinedSplit
    • decomposition.RandomizedPCA
    • gaussian_process.GaussianProcess
    • grid_search.ParameterGrid
    • grid_search.ParameterSampler
    • grid_search.GridSearchCV
    • grid_search.RandomizedSearchCV
    • mixture.DPGMM
    • mixture.GMM
    • mixture.VBGMM

  • from whats new reported here

    • Linear, kernelized and related models

      • residual_metric has been deprecated in :class:linear_model.RANSACRegressor. Use loss instead. By Manoj Kumar_.
      • Access to public attributes .X_ and .y_ has been deprecated in :class:isotonic.IsotonicRegression. By :user:Jonathan Arfa <jarfa>.

    • Decomposition, manifold learning and clustering

      • The old :class:mixture.DPGMM is deprecated in favor of the new :class:mixture.BayesianGaussianMixture (with the parameter weight_concentration_prior_type='dirichlet_process'). The new class solves the computational problems of the old class and computes the Gaussian mixture with a Dirichlet process prior faster than before. :issue:7295 by :user:Wei Xue <xuewei4d> and :user:Thierry Guillemot <tguillemot>.
      • The old :class:mixture.VBGMM is deprecated in favor of the new :class:mixture.BayesianGaussianMixture (with the parameter weight_concentration_prior_type='dirichlet_distribution'). The new class solves the computational problems of the old class and computes the Variational Bayesian Gaussian mixture faster than before. :issue:6651 by :user:Wei Xue <xuewei4d> and :user:Thierry Guillemot <tguillemot>.
      • The old :class:mixture.GMM is deprecated in favor of the new :class:mixture.GaussianMixture. The new class computes the Gaussian mixture faster than before and some of computational problems have been solved. :issue:6666 by :user:Wei Xue <xuewei4d> and :user:Thierry Guillemot <tguillemot>.

    • Model evaluation and meta-estimators

      • The :mod:sklearn.cross_validation, :mod:sklearn.grid_search and :mod:sklearn.learning_curve have been deprecated and the classes and functions have been reorganized into the :mod:sklearn.model_selection module. Ref :ref:model_selection_changes for more information. :issue:4294 by Raghav RV_.

      • The grid_scores_ attribute of :class:model_selection.GridSearchCV and :class:model_selection.RandomizedSearchCV is deprecated in favor of the attribute cv_results_. Ref :ref:model_selection_changes for more information. :issue:6697 by Raghav RV_.

      • The parameters n_iter or n_folds in old CV splitters are replaced by the new parameter n_splits since it can provide a consistent and unambiguous interface to represent the number of train-test splits. :issue:7187 by :user:YenChen Lin <yenchenlin>.

      • classes parameter was renamed to labels in :func:metrics.hamming_loss. :issue:7260 by :user:Sebastián Vanrell <srvanrell>.

      • The splitter classes LabelKFold, LabelShuffleSplit, LeaveOneLabelOut and LeavePLabelsOut are renamed to :class:model_selection.GroupKFold, :class:model_selection.GroupShuffleSplit, :class:model_selection.LeaveOneGroupOut and :class:model_selection.LeavePGroupsOut respectively. Also the parameter labels in the :func:split method of the newly renamed splitters :class:model_selection.LeaveOneGroupOut and :class:model_selection.LeavePGroupsOut is renamed to groups. Additionally in :class:model_selection.LeavePGroupsOut, the parameter n_labels is renamed to n_groups. :issue:6660 by Raghav RV_.

      • Error and loss names for scoring parameters are now prefixed by 'neg_', such as neg_mean_squared_error. The unprefixed versions are deprecated and will be removed in version 0.20. :issue:7261 by :user:Tim Head <betatim>.

files with remaining deprecated lines

  • sklearn/isotonic.py
  • sklearn/metrics/ranking.py
  • sklearn/metrics/scorer.py
  • sklearn/tree/export.py
  • sklearn/decomposition/online_lda.py
  • sklearn/cluster/hierarchical.py
  • sklearn/utils/testing.py
  • sklearn/svm/classes.py
  • sklearn/linear_model/least_angle.py
  • sklearn/linear_model/base.py
  • sklearn/tests/test_base.py
  • sklearn/model_selection/_search.py
  • sklearn/model_selection/tests/test_search.py
  • sklearn/base.py

@massich
Copy link
Contributor Author
massich commented Nov 9, 2017

Apparently docs/modules/cross_validation.rst is already using sklearn.model_selection so this is updated. Do we need to move the documentation somewhere else (like docs/modules/model_selection)? or is it fine already?

@jnothman
Copy link
Member
jnothman commented Nov 9, 2017

This pull request introduces 1 alert and fixes 3 - view on lgtm.com

new alerts:

  • 1 for Explicit export is not defined

fixed alerts:

  • 2 for Potentially uninitialized local variable
  • 1 for Non-iterable used in for loop

Comment posted by lgtm.com

@massich massich changed the title [MRG] MNT Remove cross_validation.py (deprecated in 0.18) [WIP] MNT Remove cross_validation.py (deprecated in 0.18) Nov 9, 2017
@massich massich force-pushed the remove_020_deprecated_code branch from 675c51e to a651476 Compare November 9, 2017 14:20
@jnothman
Copy link
Member
jnothman commented Nov 9, 2017

This pull request fixes 3 alerts - view on lgtm.com

fixed alerts:

  • 2 for Potentially uninitialized local variable
  • 1 for Non-iterable used in for loop

Comment posted by lgtm.com

@massich massich changed the title [WIP] MNT Remove cross_validation.py (deprecated in 0.18) [WIP] MNT Remove code deprecated in 0.18 Nov 9, 2017
@massich massich force-pushed the remove_020_deprecated_code branch from 30283eb to 6044f7f Compare November 9, 2017 16:52
@massich
Copy link
Contributor Author
massich commented Nov 9, 2017

Is there any reason to keep this?

scikit-learn/sklearn/multioutput.py

Lines 282 to 285 in 3111b8e

# XXX remove in 0.19 when r2_score default for multioutput changes
from .metrics import r2_score
return r2_score(y, self.predict(X), sample_weight=sample_weight,
multioutput='uniform_average')

@massich massich force-pushed the remove_020_deprecated_code branch from e529e0a to 9f8741d Compare November 9, 2017 17:11
@jnothman
Copy link
Member
jnothman commented Nov 9, 2017

This pull request fixes 6 alerts - view on lgtm.com

fixed alerts:

  • 2 for Non-callable called
  • 2 for Potentially uninitialized local variable
  • 1 for Loop variable capture
  • 1 for Non-iterable used in for loop

Comment posted by lgtm.com

@qinhanmin2014
Copy link
Member

@massich Thanks for the work :) Some suggestions:
(1) There's already a PR by amueller #9570 along with some concern, I suppose you have contact with him? If so, it might be better to link/close that PR in this PR.
(2) Since you expand your PR to the deprecation for 0.20, it might be better to have a checklist to explain what you have done and what you are going to do.

@massich
Copy link
Contributor Author
massich commented Nov 10, 2017

my bad. I did not see it. It was just that I was doing MNT in #10017 and I started this PR to clean up the code first ( to avoid unnecessary changes ).
Any comments @amueller ?

@massich massich force-pushed the remove_020_deprecated_code branch from 9f8741d to 2ec39c0 Compare November 10, 2017 11:59
@massich massich changed the title [WIP] MNT Remove code deprecated in 0.18 [MRG] MNT Remove code deprecated in 0.18 Nov 10, 2017
@jnothman
Copy link
Member

This pull request fixes 6 alerts - view on lgtm.com

fixed alerts:

  • 2 for Non-callable called
  • 2 for Potentially uninitialized local variable
  • 1 for Loop variable capture
  • 1 for Non-iterable used in for loop

Comment posted by lgtm.com

@massich massich force-pushed the remove_020_deprecated_code branch from 6558ae8 to ec2b65b Compare November 10, 2017 14:53
@massich massich force-pushed the remove_020_deprecated_code branch from ec2b65b to 2ffa7bd Compare November 10, 2017 15:09
massich
massich commented Nov 10, 2017
View reviewed changes
sklearn/metrics/tests/test_score_objects.py
@@ -499,25 +499,6 @@ def test_scorer_memmap_input():
yield check_scorer_memmap, name


def test_deprecated_names():
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this should go out in version 0.20 and if so, if classes.rst is correct anymore. here:

scikit-learn/doc/modules/classes.rst

Lines 779 to 805 in 01e0639

.. autosummary::
:toctree: generated/
:template: function.rst
metrics.accuracy_score
metrics.auc
metrics.average_precision_score
metrics.balanced_accuracy_score
metrics.brier_score_loss
metrics.classification_report
metrics.cohen_kappa_score
metrics.confusion_matrix
metrics.f1_score
metrics.fbeta_score
metrics.hamming_loss
metrics.hinge_loss
metrics.jaccard_similarity_score
metrics.log_loss
metrics.matthews_corrcoef
metrics.precision_recall_curve
metrics.precision_recall_fscore_support
metrics.precision_score
metrics.recall_score
metrics.roc_auc_score
metrics.roc_curve
metrics.zero_one_loss

Copy link
Member
@jnothman jnothman Jan 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, classes.rst is still valid. This deprecation only pertains to getting scorers by a string.

@massich
Copy link
Contributor Author
massich commented Nov 10, 2017

What do we do with these two lost reference to LavelKFold ??:

scikit-learn/doc/modules/classes.rst

Line 1413 in 01e0639

cross_validation.LabelKFold

scikit-learn/examples/model_selection/plot_nested_cross_validation_iris.py

Line 78 in 01e0639

# E.g "LabelKFold", "LeaveOneOut", "LeaveOneLabelOut", etc.

@massich massich force-pushed the remove_020_deprecated_code branch from 541c31c to b36341e Compare November 10, 2017 15:44
@jnothman
Copy link
Member

This pull request fixes 5 alerts - view on lgtm.com

fixed alerts:

  • 2 for Non-callable called
  • 1 for Loop variable capture
  • 1 for Non-iterable used in for loop
  • 1 for Potentially uninitialized local variable

Comment posted by lgtm.com

@jnothman
Copy link
Member

This pull request fixes 5 alerts - view on lgtm.com

fixed alerts:

  • 2 for Non-callable called
  • 1 for Loop variable capture
  • 1 for Non-iterable used in for loop
  • 1 for Potentially uninitialized local variable

Comment posted by lgtm.com

@qinhanmin2014
Copy link
Member

I think it might be better to finish the deprecation to avoid some unnecessary change in other PR. Since there's lots of work, maybe we can review and merge part of it every time.

@qinhanmin2014 qinhanmin2014 added this to the 0.20 milestone Jan 11, 2018
@jnothman
Copy link
Member

I would've liked it if the class and module deprecations happened first in one chunk, then work on the smaller fries...

jnothman
jnothman reviewed Jan 11, 2018
View reviewed changes
Copy link
Member
@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the entries from the "Deprecated" section in classes.rst.

doc/modules/dp-derivation.rst still references :class:`GMM`

Removed modules are still referenced in sklearn/tests/test_docstring_parameters.py

sklearn/utils/estimator_checks.py still mentions GaussianProcess.

Otherwise LGTM as a first shot. Put the others into separate issue.

benchmarks/bench_plot_incremental_pca.py
@@ -116,9 +111,7 @@ def variable_batch_size_comparison(data):
all_times = defaultdict(list)
all_errors = defaultdict(list)
pca = PCA(n_components=n_components)
rpca = RandomizedPCA(n_components=n_components, random_state=1999)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be reporting PCA(svd_solver='randomized') instead?

sklearn/metrics/scorer.py
@@ -225,18 +225,13 @@ def get_scorer(scoring):
scorer : callable
The scorer.
"""
valid = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should leave this code in in case of future deprecations. Or else just note somewhere that if deprecations are needed, SCORER can be redefined as a DeprecationDict, but keys etc then need to be reimplemented to hide deprecated keys.

sklearn/metrics/tests/test_score_objects.py
@@ -499,25 +499,6 @@ def test_scorer_memmap_input():
yield check_scorer_memmap, name


def test_deprecated_names():
Copy link
Member
@jnothman jnothman Jan 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, classes.rst is still valid. This deprecation only pertains to getting scorers by a string.

@jnothman
Copy link
Member

@massich, should we find someone else to complete this?

auvipy
auvipy suggested changes Mar 5, 2018
View reviewed changes
Copy link
@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase

@rth
Copy link
Member
rth commented May 7, 2018 •
edited
Loading

This would very helpful, do you still have some bandwidth to work on it @massich or should it be taken over?

@amueller
Copy link
Member

I might revive #9570 and/or try to merge this one with that one.

@amueller
Copy link
Member

Sorry I didn't realize this was a take-over of my PR (which was forever stalled). I can take the wheel again if @massich is busy, but either is fine for me.

@massich
Copy link
Contributor Author
massich commented May 22, 2018 via email

@amueller amueller mentioned this pull request May 22, 2018
Merged
45 tasks
@massich
Copy link
Contributor Author
massich commented Jun 5, 2018

I guess we can close this one. @amueller do you mind doing the honors?

@ogrisel
Copy link
Member
ogrisel commented Jun 6, 2018

Closing in favor or #9570.

@ogrisel ogrisel closed this Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman left review comments

@auvipy auvipy auvipy requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.20
Development

Successfully merging this pull request may close these issues.
7 participants
@massich @jnothman @qinhanmin2014 @rth @amueller @ogrisel @auvipy
0