-
Notifications
You must be signed in to change notification settings - Fork 1.3k
added AllKNN under-sampling method #97
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
Conversation
@dvro Could you also write the test? You can check the other file to check how it is done for the moment. |
You should also avoid to merge and use rebase I think. The merge will be on our side. |
Ok, I'll do that today! |
02beef0
to
4bfbe2a
Compare
@glemaitre I believe it is fine now; let me know if it still needs any improvement! |
Code wise it is fine. Just a last addition: can you add to the changelog in |
@glemaitre done updated |
I got an issue after merging on master with the testing on my repo. The table were mismatching. |
That's weird ... everything seems to be fine on my end ...
I'll look into it! |
2fe4121
to
0e643dd
Compare
5b5a016
to
28a6ee1
Compare
6671082
to
e2f6f25
Compare
currdir = os.path.dirname(os.path.abspath(__file__)) | ||
X_gt = np.load(os.path.join(currdir, 'data', 'allknn_x.npy')) | ||
y_gt = np.load(os.path.join(currdir, 'data', 'allknn_y.npy')) | ||
assert_array_equal(X_resampled, X_gt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dvro Try to use assert_array_almost_equal
to see if this is not a problem of rounding.
@glemaitre check out this weirdest thing:
and the test_allknn.py passes:
I created a clean-env using the same python version, numpy/scipy version, but the problem persists, specifically do you think you could generate the Thanks, |
@dvro I am doing that. |
@dvro done |
Add data
ok so that's pretty weird, which OS are you using? |
>>> from collections import Counter | ||
>>> from sklearn.datasets import fetch_mldata | ||
>>> from imblearn.under_sampling import AllKNN | ||
>>> pima = fetch_mldata('diabetes_scale') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the example to avoid fetching data. It can be a problem at testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@glemaitre whats up with this appveyor? it seems like several tests are failing ... If everything is ok now, I'll squeeze the commits, pull and (finally) merge. |
@dvro appveyor is still not working because of the npy file. That is why we have to move away from this solution. This is fine for merging for me. Do you think that we need |
* added AllKNN under sampling technique * test_allknn using assert_array_almost_equal * Add data * changing allknn doctest and removing internal data copy in _sample(X, y)
* added AllKNN under sampling technique * test_allknn using assert_array_almost_equal * Add data * changing allknn doctest and removing internal data copy in _sample(X, y)
* added AllKNN under sampling technique * test_allknn using assert_array_almost_equal * Add data * changing allknn doctest and removing internal data copy in _sample(X, y)
* added AllKNN under sampling technique * test_allknn using assert_array_almost_equal * Add data * changing allknn doctest and removing internal data copy in _sample(X, y)
python plot_allknn.py