-
Notifications
You must be signed in to change notification settings - Fork 21
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
Ordering methods are inconsistent for Double and Float #10511
Comments
I've discovered that the problem is worse than I originally thought, so I've updated the issue description accordingly. |
As I see it, there are three options here:
I am in favour of option 2, but I'm happy to make and PR changes based on any of them. |
Extended testing leaves me convinced that option 3 is extremely difficult, if not impossible, to do correctly. I'm happy for someone to prove me wrong though. |
I've dug up the original source of this problem, which seems to be #5104, and this commit. While the commit does make |
Remove overridden method implementations in Ordering. Fixes scala/bug#10511
Remove overridden method implementations in Ordering. Reverts 460bbc1 Fixes scala/bug#10511
Remove overridden method implementations in Ordering. Reverts 460bbc1 Fixes scala/bug#10511
See also scala/scala#76 |
Remove overridden method implementations in Ordering. Reverts 460bbc1 Fixes scala/bug#10511
Remove overridden method implementations in Ordering. Reverts 460bbc1 Fixes scala/bug#10511
I've discovered more problems caused by these inconsistencies regarding |
This comment has been minimized.
This comment has been minimized.
for reference, most of the discussion happened at scala/scala#6323 and scala/scala#6410 (I'm mostly leaving this comment because I keep forgetting where they took place myself) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
good catch there Seth - this is a nightmarishly bad first issue 😄 |
IMO a new issue should be created and all messages since #10511 (comment) should be transferred to the new issue. It makes no sense to me to reopen this issue and refix it. The current issue was fixed in 2.13.0-M5 already. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I opened #11844, so I'm closing this issue here. |
Fixes scala/bug#11844 Ref scala/bug#10511 Ref scala#6410 Ref scala#76 This change the deprecation of `DeprecatedDoubleOrdering` to a migration warning instead to avoid `List(1.0, -1.0).sorted` giving deprecation warning. This also provides some documentation on the ordering instances in Scaladoc.
Fixes scala/bug#11844 Ref scala/bug#10511 Ref scala#6410 Ref scala#76 This change the deprecation of `DeprecatedDoubleOrdering` to a migration warning instead to avoid `List(1.0, -1.0).sorted` giving deprecation warning. This also provides some documentation on the ordering instances in Scaladoc.
For:
Float.NaN
Double.NaN
-0.0f
and0.0f
-0.0d
and0.0d
the following inconsistencies exist:
Ordering.lt
can return false whenOrdering.compare
returns a negative numberOrdering.lteq
can return false whenOrdering.compare
returns a negative number or0
Ordering.equiv
can return false whenOrdering.compare
returns0
Ordering.equiv
can return true whenOrdering.compare
returns a non-zero valueOrdering.gteq
can return false whenOrdering.compare
returns a positive number or0
Ordering.gt
can return false whenOrdering.compare
returns a positive numberThe reason for the inconsistency is that the convenience methods are overridden to use the primitive operators (
<
,<=
,==
,>=
, and>
respectively), which always return false for operations where at one of the operands isNaN
. Howevercompare
is implemented byjava.lang.{Float, Double}.compare
, for whichNaN
is ordered as larger than all other values. Also, the primitive operators treat-0.0
and0.0
as equal, whilejava.lang.{Float, Double}.compare
do not.Ordering.min
is inconsistent withcompare
as well; however, I'm not sure it is consistent with the other methods either.Ordering.max
seems to be consistent withcompare
by chance.It is possible that this behavior is intended, though counter-intuitive, but if so, it should probably be documented.
One result of these inconsistencies is that
TraversableOnce.min
and.max
are non-deterministic (and wrong):Those methods could be fixed by using
Ordering.min
and.max
internally, instead of.lteq
and.gteq
; however, they shouldn't need to.The text was updated successfully, but these errors were encountered: