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

`FromJSON` and `ToJSON` don't roundtrip for `Double` #679

Open
NorfairKing opened this Issue Nov 6, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@NorfairKing
Copy link

NorfairKing commented Nov 6, 2018

1) Test.Validity.Aeson, JSON Double (unchecked), decode :: Data.ByteString.Lazy.ByteString -> Either String Double, ensures that encode and decode are inverses for "unchecked Double"'s
     Falsifiable (after 12 tests):
       Infinity
     Decoding succeeded, but the decoded value
     NaN
     differs from expected decoded value
     Infinity
     'encode' encoded it to the json
     "null"
@NorfairKing

This comment has been minimized.

Copy link

NorfairKing commented Nov 6, 2018

The same holds for `-Infinity:

  src/Test/Validity/Aeson.hs:135:21: 
  1) Test.Validity.Aeson, JSON Double (sequence of 'a's), decode :: Data.ByteString.Lazy.ByteString -> Either String Double, ensures that encode and decode are inverses for "sequence of 'a's Double"'s
       Falsifiable (after 1 test):
         -Infinity
       Decoding succeeded, but the decoded value
       NaN
       differs from expected decoded value
       -Infinity
       'encode' encoded it to the json
       "null"
@NorfairKing

This comment has been minimized.

Copy link

NorfairKing commented Nov 6, 2018

It's also broken the different values of NaN if you ignore that NaN /= NaN` holds but feel free to ignore that if you don't care about NaN payloads.

@Lysxia

This comment has been minimized.

Copy link
Contributor

Lysxia commented Nov 6, 2018

I wonder how this should be fixed. This ECMA spec just says:

Numeric values that cannot be represented as sequences of digits (such as Infinity and NaN) are not
permitted.

Looking around, I can find no consensus about how to handle this, because at some point you'll want to encode a double which may be one of these special values

https://stackoverflow.com/questions/1423081/json-left-out-infinity-and-nan-json-status-in-ecmascript

Though silently encoding to null seems a pretty common choice to avoid throwing exceptions.

capnproto/capnproto#261
graphite-project/graphite-web#813

If we really want roundtripping for infinity values, maybe we should encode them as strings.

@NorfairKing

This comment has been minimized.

Copy link

NorfairKing commented Nov 6, 2018

@Lysxia I'm fine with just throwing up our hands and saying "here are al the links to related projects that are similarly broken as intended" by the way. I just wanted to point it out because it showed up in my test suite.

@phadej

This comment has been minimized.

Copy link
Contributor

phadej commented Nov 6, 2018

@quasicomputational

This comment has been minimized.

Copy link

quasicomputational commented Nov 6, 2018

I agree that the quote from the spec isn't the final word and stringy encoding would make sense. It's discussing JSON's concept of numbers; there's no rule that applications can't interpret, e.g., the string 'NaN' as NaN, just that a JSON processor has to reject NaN as a numeric value. Obviously this means that we won't inter-operate with anything that doesn't use the same conventions, but we're currently not able to inter-operate with anything that needs NaNs and infinities transmitted anyway.

@phadej

This comment has been minimized.

Copy link
Contributor

phadej commented Nov 6, 2018

As we can differentiate between

λ> signum (-0.0 :: Double)
-0.0
λ> signum (0.0 :: Double)
0.0

how we would encode -0.0?

Double and equality makes my head hurt. I guess encoding infinities as "inf", "ninf" and leaving null for NaN won't break many users.

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