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.

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

[MRG+1] EHN: scikit-learn API transition #462

Merged
merged 10 commits into from Aug 28, 2018

Conversation

Projects
None yet
4 participants
@glemaitre
Member

glemaitre commented Aug 27, 2018

closes #460

This PR implements:

  • Removing sample.
  • Having a single fit_resample. In addition, we kept an alias fit_sample for backcompatibility.
  • Sample X and y and any additional array of the same size.
@pep8speaks

This comment has been minimized.

pep8speaks commented Aug 27, 2018

Hello @glemaitre! Thanks for updating the PR.

Line 27:1: E402 module level import not at top of file
Line 28:1: E402 module level import not at top of file
Line 29:1: E402 module level import not at top of file
Line 51:1: E402 module level import not at top of file
Line 52:1: E402 module level import not at top of file
Line 53:1: E402 module level import not at top of file
Line 54:1: E402 module level import not at top of file
Line 55:1: E402 module level import not at top of file
Line 56:1: E402 module level import not at top of file
Line 86:1: E402 module level import not at top of file
Line 93:1: E402 module level import not at top of file
Line 94:1: E402 module level import not at top of file
Line 128:1: E402 module level import not at top of file
Line 129:1: E402 module level import not at top of file
Line 148:1: E402 module level import not at top of file
Line 163:1: E402 module level import not at top of file
Line 185:1: E402 module level import not at top of file
Line 220:1: E402 module level import not at top of file
Line 221:1: E402 module level import not at top of file

Comment last updated on August 27, 2018 at 22:06 Hours UTC
@glemaitre

This comment has been minimized.

Member

glemaitre commented Aug 27, 2018

@chkoar This will be a huge one difficult to review :)

I tried to keep it lean.

@glemaitre

This comment has been minimized.

Member

glemaitre commented Aug 27, 2018

If you can have a look at it.

glemaitre added some commits Aug 27, 2018

@codecov

This comment has been minimized.

codecov bot commented Aug 27, 2018

Codecov Report

Merging #462 into master will decrease coverage by 0.04%.
The diff coverage is 99.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #462      +/-   ##
==========================================
- Coverage   98.78%   98.74%   -0.05%     
==========================================
  Files          75       75              
  Lines        4628     4543      -85     
==========================================
- Hits         4572     4486      -86     
- Misses         56       57       +1
Impacted Files Coverage Δ
imblearn/utils/tests/test_validation.py 100% <ø> (ø) ⬆️
imblearn/utils/__init__.py 100% <ø> (ø) ⬆️
...pling/_prototype_selection/_one_sided_selection.py 100% <100%> (ø) ⬆️
imblearn/datasets/_imbalance.py 100% <100%> (ø) ⬆️
...sampling/_prototype_selection/tests/test_allknn.py 100% <100%> (ø) ⬆️
imblearn/over_sampling/tests/test_adasyn.py 100% <100%> (ø) ⬆️
imblearn/ensemble/tests/test_balance_cascade.py 100% <100%> (ø) ⬆️
imblearn/over_sampling/tests/test_smote.py 100% <100%> (ø) ⬆️
...ling/_prototype_selection/_random_under_sampler.py 100% <100%> (ø) ⬆️
imblearn/combine/_smote_tomek.py 100% <100%> (ø) ⬆️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10c4196...6189206. Read the comment docs.

glemaitre added some commits Aug 27, 2018

@mrastgoo

This comment has been minimized.

mrastgoo commented Aug 27, 2018

add # noqa in the imports of the notebook style examples so that PEP8 stop complaining

@massich

This comment has been minimized.

Contributor

massich commented Aug 27, 2018

Implement the last part in a separate PR (and link to this one)

glemaitre added some commits Aug 27, 2018

@glemaitre

This comment has been minimized.

Member

glemaitre commented Aug 28, 2018

@chkoar @massich @mrastgoo Any reviews?

@massich massich changed the title from EHN: scikit-learn API transition to [MRG+1] EHN: scikit-learn API transition Aug 28, 2018

@massich massich merged commit e49c30a into scikit-learn-contrib:master Aug 28, 2018

6 checks passed

LGTM analysis: Python 2 fixed alerts
Details
ci/circleci: python3 Your tests passed on CircleCI!
Details
codecov/patch 99.3% of diff hit (target 98.78%)
Details
codecov/project Absolute coverage decreased by -0.04% but relative coverage increased by +0.51% compared to 10c4196
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment