Skip to content
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] Better in-out support #681

Merged
merged 9 commits into from
Feb 12, 2020

Conversation

chkoar
Copy link
Member

@chkoar chkoar commented Feb 3, 2020

Reference Issue

Follows #673. Fixes #666.

What does this implement/fix? Explain your changes.

Adds better support for pandas and simple lists, if this is the case.
Gradually we could support more array data types.

Any other comments?

I created new type for the job just to keep some state.
The intention is to be private, thus no tested are provided.
Existing tests are passing, at least on my machine. Let's see what the CIs think.

@pep8speaks
Copy link

pep8speaks commented Feb 3, 2020

Hello @chkoar! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-02-07 14:21:40 UTC

@codecov
Copy link

codecov bot commented Feb 3, 2020

Codecov Report

Merging #681 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   98.59%   98.65%   +0.05%     
==========================================
  Files          82       82              
  Lines        4850     4907      +57     
==========================================
+ Hits         4782     4841      +59     
+ Misses         68       66       -2
Impacted Files Coverage Δ
imblearn/over_sampling/_smote.py 97.21% <ø> (+0.51%) ⬆️
imblearn/over_sampling/_random_over_sampler.py 100% <ø> (ø) ⬆️
...ling/_prototype_selection/_random_under_sampler.py 100% <ø> (ø) ⬆️
...n/under_sampling/_prototype_selection/_nearmiss.py 98.59% <100%> (ø) ⬆️
imblearn/utils/_validation.py 100% <100%> (ø) ⬆️
imblearn/utils/estimator_checks.py 96.68% <100%> (+0.38%) ⬆️
imblearn/utils/tests/test_validation.py 100% <100%> (ø) ⬆️

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 42cd496...c5f40b4. Read the comment docs.

@chkoar chkoar force-pushed the better_in_out_support branch from e77b8ab to 35d7af9 Compare February 3, 2020 12:09
@chkoar chkoar requested a review from glemaitre February 3, 2020 12:12
@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2020

This pull request introduces 1 alert when merging 35d7af9 into 3ede269 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@lgtm-com
Copy link

lgtm-com bot commented Feb 3, 2020

This pull request introduces 1 alert when merging 182dc6e into 3ede269 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@chkoar chkoar changed the title Better in-out support for pandas [MRG] Better in-out support Feb 3, 2020
imblearn/utils/_validation.py Outdated Show resolved Hide resolved
imblearn/utils/_validation.py Show resolved Hide resolved
imblearn/utils/_validation.py Show resolved Hide resolved
@chkoar chkoar requested a review from glemaitre February 5, 2020 09:49
@chkoar
Copy link
Member Author

chkoar commented Feb 7, 2020

@glemaitre I think that there is no need to attach that object to the the sampler instance. So, when you have time check again.

@chkoar chkoar requested a review from glemaitre February 7, 2020 14:10
@glemaitre glemaitre merged commit 4ba2803 into scikit-learn-contrib:master Feb 12, 2020
@glemaitre
Copy link
Member

This is good

@glemaitre
Copy link
Member

Arfff I forgot to check the what's new. Could you make an entry in what's new in 0.6.2
I will make probably a bug fix release because we add quite a lot of feedback on the bug in #666

@chkoar
Copy link
Member Author

chkoar commented Feb 13, 2020

@glemaitre see #682 and update it if you want.

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.

AttributeError: 'DataFrame' object has no attribute 'name'
3 participants