basvandijk / scientific

Arbitrary-precision floating-point numbers represented using scientific notation
BSD 3-Clause "New" or "Revised" License
73 stars 40 forks source link

Unexpected reads behavior for scientific #21

Closed timbod7 closed 9 years ago

timbod7 commented 10 years ago

My parsing code fails because reads for scientific has different behaviour to the builtin types:

$ ghci
GHCi, version 7.8.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> reads "" :: [(Int,String)]
[]
Prelude> reads "" :: [(Double,String)]
[]
Prelude> reads "" :: [(Data.Scientific.Scientific,String)]
Loading package array-0.5.0.0 ... linking ... done.
Loading package deepseq-1.3.0.2 ... linking ... done.
Loading package bytestring-0.10.4.0 ... linking ... done.
Loading package text-1.1.0.0 ... linking ... done.
Loading package hashable-1.2.2.0 ... linking ... done.
Loading package scientific-0.3.3.1 ... linking ... done.
[(0.0,"")]
Prelude> 

Is this intended?

basvandijk commented 10 years ago

Thanks for the report!

I released a fix.

timbod7 commented 10 years ago

Thanks for the quick response. The reads parsing still differs from Double:

$ ghci
GHCi, version 7.8.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> :m Data.Scientific 
Prelude Data.Scientific> reads "" :: [(Double,String)]
[]
Prelude Data.Scientific> reads "" :: [(Data.Scientific.Scientific,String)]
Loading package array-0.5.0.0 ... linking ... done.
Loading package deepseq-1.3.0.2 ... linking ... done.
Loading package bytestring-0.10.4.0 ... linking ... done.
Loading package text-1.1.0.0 ... linking ... done.
Loading package hashable-1.2.2.0 ... linking ... done.
Loading package scientific-0.3.3.2 ... linking ... done.
[]
Prelude Data.Scientific> reads "0.0" :: [(Double,String)]
[(0.0,"")]
Prelude Data.Scientific> reads "0.0" :: [(Data.Scientific.Scientific,String)]
[(0.0,".0"),(0.0,"")]
Prelude Data.Scientific> 
basvandijk commented 10 years ago

Mmm, arguably this is correct behaviour but I agree that we should match the Double read semantics.

I don't have much time to come up with a fix this week but I'll do my best. Feel feel to solve it yourself and send me a pull request (preferably with test cases) :-)

timbod7 commented 10 years ago

I see what you mean. Perhaps I shouldn't be relying on reads. I need a parsec parser for Data.Scientific, to replace one I have for Double. I currently have:

  pread :: ReadS a -> P.Parser a
  pread reads = do
    s <- P.getInput
    case reads (T.unpack s) of
       [(v,s')] -> (v <$ P.setInput (T.pack s'))
       _ -> empty

  parseDouble :: Parser Double
  parseDouble = pread reads

  parseScientific :: Parser Scientific
  parseScientific = pread reads

This works, for Double, but not for Scientific, due to the partial results it returns.

Is there a better way?

On Mon, Oct 27, 2014 at 9:57 PM, Bas van Dijk notifications@github.com wrote:

Reopened #21 https://github.com/basvandijk/scientific/issues/21.

— Reply to this email directly or view it on GitHub https://github.com/basvandijk/scientific/issues/21#event-183980384.

basvandijk commented 10 years ago

You could perhaps take the attoparsec parser for Scientific and adapt it to parsec. On 27 Oct 2014 12:03, "Tim Docker" notifications@github.com wrote:

I see what you mean. Perhaps I shouldn't be relying on reads. I need a parsec parser for Data.Scientific, to replace one I have for Double. I currently have:

pread :: ReadS a -> P.Parser a pread reads = do s <- P.getInput case reads (T.unpack s) of [(v,s')] -> (v <$ P.setInput (T.pack s')) _ -> empty

parseDouble :: Parser Double parseDouble = pread reads

parseScientific :: Parser Scientific parseScientific = pread reads

This works, for Double, but not for Scientific, due to the partial results it returns.

Is there a better way?

On Mon, Oct 27, 2014 at 9:57 PM, Bas van Dijk notifications@github.com wrote:

Reopened #21 https://github.com/basvandijk/scientific/issues/21.

— Reply to this email directly or view it on GitHub https://github.com/basvandijk/scientific/issues/21#event-183980384.

— Reply to this email directly or view it on GitHub https://github.com/basvandijk/scientific/issues/21#issuecomment-60576833 .

timbod7 commented 10 years ago

I can look at that. I was hoping to leverage a builtin parser rather than having to write/convert one from scratch. My workaround for now is to parse a Double, and then convert that to the Scientifc. This works, but isn't great.

neongreen commented 10 years ago

I have a similar issue – read " 8" :: Scientific fails, while read " 8" :: Double succeeds. It's worse than it sounds, because it means that read "Just 8" :: Maybe Scientific doesn't work either.

basvandijk commented 9 years ago

This is fixed in 0.3.3.7.