Closed
Description
The slices.Sort
doesn't support sorting slices of floating-point numbers containing Not-a-number (NaN) values, It's not very intuitive.
Once the slices
package is merged into stdlib, the sort.{TYPE}s
(e.g. sort.Ints
, sort.Float64s
) APIs may be semi-deprecated. But the slices
package doesn't provide a simple way to sort slices of floats containing NaNs, it recommends using slices.SortFunc(x, func(a, b float64) bool {return a < b || (math.IsNaN(a) && !math.IsNaN(b))})
, this function is very complex compared to using sort.Float64s
.
Related CL: https://go-review.googlesource.com/c/exp/+/446155
Metadata
Metadata
Assignees
Type
Projects
Relationships
Development
No branches or pull requests
Activity
gopherbot commentedon Oct 31, 2022
Change https://go.dev/cl/446155 mentions this issue:
slices: Sort support sorting slices of floating-point numbers containing NaN values
dsnet commentedon Oct 31, 2022
This is related to #33440. @smasher164 mentions the existence of a total-ordering predicate that could be used to sort floats with NaNs.
ianlancetaylor commentedon Oct 31, 2022
CC @eliben @randall77
randall77 commentedon Oct 31, 2022
I don't think they will, using
sort.Float64s
will be fine forever.I think the only advantage of
slices.Sort
is that it will work on slices of namedfloat64
s (e.g.type F float64; a := []F{5,4,3}; slices.Sort(a)
, that won't work usingsort.Float64s
). And in that rare case one can use the described workaround.zhangyunhao116 commentedon Nov 1, 2022
Since Go1.18 is released, many users use the
slices.Sort
to replacesort.{TYPE}s
, because the former is faster and more clear in almost all cases. From the point of view of the users, the former looks more like a replacement of the latter, they have almost the same function in fact except theslices.Sort
doesn't support sorting slices containing NaNs.The main concern is that users don't notice this special case, and the replacement may break their codes.
earthboundkid commentedon Nov 2, 2022
I think it's worth adding documentation to slices.Sort that sorting a float won't work. Maybe also there could be a comparator function in sort.
eliben commentedon Nov 2, 2022
The documentation for
Sort
is currently:Is this not sufficient documentation, @carlmjohnson ?
eliben commentedon Nov 2, 2022
@zhangyunhao116 I'm not a big fan of this proposal at this time. The problem with sorting floats is clearly documented and a workaround is provided. It's easy to sort using
SortFunc
earthboundkid commentedon Nov 2, 2022
That seems good to me.
In #50340 I think there was talk about adding sort.Compare which would take a comparable and return -1, 0, 1. If something like that is ever added there should also be CompareFloat, but it's not necessary on its own.
zhangyunhao116 commentedon Nov 4, 2022
Agree that the special case is clearly documented and has a workaround, but it's nice to support all the special cases as
sort.{TYPE}s
does :)I did a simple poll about
slices.Sort
at my work. The question is51 people joined it, 82.4% (42) selected A, and 17.6% (9) selected B. Looks like many users of this API doesn't notice the special case.
zhangyunhao116 commentedon Nov 4, 2022
The proposal still need more discussions, it's a rare case, and It's merely not as users intuitively expected. The
slices.Sort
work very well for all the general cases.63 remaining items