[MRG+2] make sure that AUC is always a float#6225
[MRG+2] make sure that AUC is always a float#6225GaelVaroquaux merged 3 commits intoscikit-learn:masterfrom
Conversation
There was a problem hiding this comment.
wait, why is this needed here?
Or rather: why is this only needed here, and not in all scorers?
There was a problem hiding this comment.
The memmap.sum() used internally in trapz does not convert to scalar float (it returns a 0d memmap singleton) while it does convert to a float scaler for regular numpy arrays. Other scorers do not have this issue as highlighted by the test.
There was a problem hiding this comment.
Should we report this to the NumPy guys?
|
I have introduced new |
f40e1ce to
a9ad70c
Compare
|
Ready for merge on my end. |
sklearn/metrics/ranking.py
Outdated
There was a problem hiding this comment.
We don't we always convert to float? Right now we'll have numpy floats or floats depending on the parallel computing setting, which might surprise people (different set of methods on the different objects).
There was a problem hiding this comment.
Not sure that's the best way to do it but I found:
area = area.dtype.type(area)or
area = area[()]to return a numpy float rather than a Python float.
There was a problem hiding this comment.
Actually I could do:
if isinstance(area, np.memmap):
area = area.dtype.type(area)This way the behavior stays consistent for ndarray and memmap instances whatever the dtype.
|
Aside from the one comment, +1 for merge. |
d81dc43 to
a13460c
Compare
|
Ok I addressed @GaelVaroquaux's comment. @amueller merge? |
a13460c to
3427d20
Compare
|
Rebased to resolve changelog conflicts. @MechCoder are you still around for a second review? |
|
lgmt as well ! |
[MRG+2] make sure that AUC is always a float
Even when computed on memory mapped data as reported in #6147.
Let's make sure that the new tests pass under AppVeyor before merging as deletion of mmap files tends to be troublesome under Windows.