-
Notifications
You must be signed in to change notification settings - Fork 102
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
Unit tests for different subsamples #468
Unit tests for different subsamples #468
Conversation
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.
Thank you for this PR! Great! @BaptisteCalot
Note that the outputs should be of the same style in MAPIE. So the correction should not be applied here but rather directly in the original file. Note that you should make sure that if they are changes, we should consider why they occur and if they can be resolved without forcing the typing.
mapie/tests/test_subsample.py
Outdated
length=length, random_state=0) | ||
trains = [x[0] for x in cv.split(X)] | ||
tests = [x[1] for x in cv.split(X)] | ||
for i in range(n_resamplings): |
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.
Don't forget to use combinations
instead of the loop! :)
mapie/tests/test_subsample.py
Outdated
) | ||
trains2 = [x[0] for x in list(cv2.split(X))] | ||
tests2 = [x[1] for x in list(cv2.split(X))] | ||
np.testing.assert_array_equal(trains1, trains2) |
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.
I'd rather have the same test i.e. assert_equal
also here
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #468 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 39 41 +2
Lines 4616 5106 +490
Branches 487 871 +384
==========================================
+ Hits 4616 5106 +490 ☔ View full report in Codecov by Sentry. |
Description
This PR aims to create unit tests to verify that the train and test sets of the split method from Subsample and BlockBootstrap classes are different even though the random state is the same
Fix issue #290
Type of change
How Has This Been Tested?
Checklist : OK
make lint
make type-check
make tests
make coverage
make doc