Skip to content

MNT simple deprecations and removals for 0.21#12238

Merged
amueller merged 12 commits intoscikit-learn:masterfrom
amueller:simple_deprecations_021
Oct 11, 2018
Merged

MNT simple deprecations and removals for 0.21#12238
amueller merged 12 commits intoscikit-learn:masterfrom
amueller:simple_deprecations_021

Conversation

@amueller
Copy link
Copy Markdown
Member

@amueller amueller commented Oct 1, 2018

Part of #11992.
These were all the things that seemed pretty straight-forward. It's actually a bit bulky but should still be easy to review, hopefully.

@sklearn-lgtm
Copy link
Copy Markdown

This pull request introduces 17 alerts and fixes 2 when merging c56c2af into 59b15c5 - view on LGTM.com

new alerts:

  • 16 for Unused import
  • 1 for Explicit export is not defined

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

This pull request introduces 17 alerts and fixes 2 when merging cd48d6d into 59b15c5 - view on LGTM.com

new alerts:

  • 16 for Unused import
  • 1 for Explicit export is not defined

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

This pull request fixes 2 alerts when merging 5069dcf into 59b15c5 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

This pull request fixes 2 alerts when merging cfa9216 into 7166cd5 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

This pull request fixes 2 alerts when merging 45289e8 into dfd009d - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

LGTM, nice to remove all this code! Only checked what's included though, if anything is missing it could be worth doing a follow up PR.

@amueller
Copy link
Copy Markdown
Member Author

amueller commented Oct 2, 2018

Yes, it's a bit hard to check for completeness - this is also not complete in the sense that it misses (at least) 4 major things (#12239 #12240 #12241 + non_negative) that looked slightly non-trivial.
I think doing this in increments makes sense.

@sklearn-lgtm
Copy link
Copy Markdown

This pull request fixes 2 alerts when merging 3afc42b into dfd009d - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

This pull request fixes 2 alerts when merging 84d6cf3 into 63e5ae6 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable

Comment posted by LGTM.com

Copy link
Copy Markdown
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.

I'm not checking that these are unused/undocumented. Let's hope that was done at time of deprecation. The code changes seem sound.

@jnothman jnothman changed the title simple deprecations and removals for 0.21 MNT simple deprecations and removals for 0.21 Oct 8, 2018
@amueller
Copy link
Copy Markdown
Member Author

should I merge now? Should be fine, right? Or wait for 0.20.1? I don't think it should conflict too much?

@jnothman
Copy link
Copy Markdown
Member

I doubt it should affect 0.20.1.

(Though we do need to get onto making that release happen sooner rather than later.)

@amueller amueller merged commit 0f94f29 into scikit-learn:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants