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

Double.NaN.signum returns 0 #11386

Open
kamilkloch opened this Issue Jan 31, 2019 · 8 comments

Comments

Projects
None yet
5 participants
@kamilkloch
Copy link

kamilkloch commented Jan 31, 2019

println(java.lang.Math.signum(Double.NaN)) //NaN
println(scala.math.signum(Double.NaN)) //NaN
println(Double.NaN.signum) //0

What is the rationale? RichDouble.scala line 48:

override def signum: Int               = math.signum(self).toInt  // !!! NaN
@Jasper-M

This comment has been minimized.

Copy link
Member

Jasper-M commented Jan 31, 2019

Seems to me that signum in scala.runtime.ScalaNumberProxy should have type T instead of Int.

@SethTisue SethTisue added this to the Backlog milestone Jan 31, 2019

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Jan 31, 2019

I think a fix could target 2.12.x nope, 👇

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Jan 31, 2019

I think a fix could target 2.12.x

I don't think so. The signature needs to be changed to returning a Double, which is definitely not a binary-compatible change.

@SethTisue

This comment has been minimized.

Copy link
Member

SethTisue commented Jan 31, 2019

in that case, we should rush a fix in. volunteer?

@SethTisue SethTisue modified the milestones: Backlog, 2.13.0-RC1 Jan 31, 2019

@kamilkloch

This comment has been minimized.

Copy link
Author

kamilkloch commented Jan 31, 2019

I'll give it a shot.

@kamilkloch

This comment has been minimized.

Copy link
Author

kamilkloch commented Feb 3, 2019

scala.runtime.ScalaNumberProxy.signum cannot return T - as a result, scala.runtime.RichByte.signumwould have to return Byte. Any ideas?

@Ichoran

This comment has been minimized.

Copy link

Ichoran commented Feb 3, 2019

I think the existing behavior, weird though it is, is as correct as you can get with a generic interface that doesn't explicitly provide for NaN eating all values and always returning false. You run into a similar problem when considering using compare to order values based on NaN. You would think that a < b if and only if a.compare(b) < 0. Also, a > b if and only if a.compare(b) > 0. But throw NaN into the mix and there's no way to achieve this.

I think the way to fix this is to leave the NumberProxy stuff alone, at a lower priority, and to explicitly have a higher-priority signum extension method for Double that gives the correct behavior for Double. You end up equally confused when working generically but you always end up confused when working generically; this at least recovers the expected behavior when you know you have a Double.

@kamilkloch

This comment has been minimized.

Copy link
Author

kamilkloch commented Feb 14, 2019

@Ichoran Pls take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment