Skip to content

[MRG+1] FIX increase tolerance of class weight check for OS X #5626

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

Merged

Conversation

ogrisel
Copy link
Member

@ogrisel ogrisel commented Oct 30, 2015

This is a fix for #5625.

Although I am not 100% to understand why we need to increase the tolerance here. I think this test used to pass on Monday. Will do a bisect and report back.

@ogrisel
Copy link
Member Author

ogrisel commented Oct 30, 2015

Will do a bisect and report back.

Actually my bad: I now remember that I was running them only on 0.17.X branch on monday on this laptop. This test did not exist on that branch hence the lack of failure at that time.

@ogrisel
Copy link
Member Author

ogrisel commented Oct 30, 2015

@TomDLT you might want to have a look.

@TomDLT
Copy link
Member

TomDLT commented Oct 30, 2015

For the record, the failing test is new in #5008, merged only 8 days ago,
so it seems OK to me to decrease the tolerance for OS X.

@TomDLT TomDLT changed the title [MRG] FIX increase tolerance of class weight check for OS X [MRG+1] FIX increase tolerance of class weight check for OS X Oct 30, 2015
@ogrisel
Copy link
Member Author

ogrisel commented Oct 31, 2015

Second review for this quick fix? @amueller @GaelVaroquaux ?

@giorgiop
Copy link
Contributor

I haven't read all #5008, but why do we need to decrease tolerance?

@amueller
Copy link
Member

amueller commented Nov 1, 2015

@giorgiop because it doesn't pass on os X.

amueller added a commit that referenced this pull request Nov 1, 2015
[MRG+1] FIX increase tolerance of class weight check for OS X
@amueller amueller merged commit bac5e39 into scikit-learn:master Nov 1, 2015
@amueller
Copy link
Member

amueller commented Nov 1, 2015

backported as 6f69e83

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.

4 participants