Closed bergmark closed 10 years ago
Hi Adam, I'm not sure what those functions are supposed to do (what are their types?). However, if you want to convert a Double to a Scientific you can use fromFloatDigits
. Converting integrals like Ints can be done using fromIntegral
.
Although it looks like those function convert a Double or Int directly to a JSON Value. Why can't you use toJSON
directly?
I was vague, and messed that up, sorry!
Aeson has the Number !Scientific
constructor for Value
, I'd like to inspect this and see if Scientific contains an Int or if it has decimal places (going the other way, to Scientific, is straight forward). We're using it with the pretty
library so the old code looks like this:
pp_value :: Value -> Doc
pp_value v = case v of
Number (I n) -> (integer :: Integer -> Doc) n
Number (D d) -> (double :: Double -> Doc) d
and I suppose I'd like a function like isIntegral :: Scientific -> Maybe Integer
since these cases are combined now. I didn't see anything immediately helpful in the docs, Is there a preferred way of doing this? And as an extension, is it then possible to make a distinction between 123
and 123.0
?
If the base10Exponent is negative you have a floating point number if it's positive its an integer.
In aeson in fromScientific we use this to either render to a floating point or to an integer number.
Thanks Bas!
For the record I ended up with
fromFloatDigits :: Double -> Scientific
fromIntegral :: Integer -> Scientific
parseNumber :: Scientific -> Either Integer Double
parseNumber n
| base10Exponent n >= 0 -> Left $ round n
| otherwise -> Right $ fromRational $ toRational n
Last line is a bit indirect, but it works :-)
Cheerio
That's good to hear Adam, some remarks:
c * 10 ^ e
will be more efficient than round
.realToFrac
is defined as fromRational . toRational
So you can refactor your parseNumber
to:
parseNumber :: Scientific -> Either Integer Double
parseNumber n
| e >= 0 -> Left $ c * 10 ^ e
| otherwise -> Right $ realToFrac n
where
e = base10Exponent n
c = coefficient n
Thanks! I'll update my code :-)
that sounds like a very nice and easy function. would you consider including it in scientific?
i wrote about the same function today, so this is another place with that function.
that sounds like a very nice and easy function. would you consider including it in scientific?
Yes, I think it makes sense to add it. Although we probably want to generalize the Double to any Fractional number as in:
parseNumber :: (Fractional a) => Scientific -> Either Integer a
And maybe we should also generalize the Integer to any Num:
parseNumber :: (Num integer, Fractional fractional)
=> Scientific -> Either integer fractional
What about the name? "parseNumber" doesn't seem right to me.
fromScientific is what i used in my package. unfortunately, that sounds too much like fromRational which has a very different type signature.
naming is hard.
I agree that parseNumber
is bad. But yeah, naming is hard.
Another option is to make it parseNumber :: Num n => Scientific -> Maybe n
since the realToFrac
case is so simple.
I kind of like the idea of constraining the integral part to Integral
even though it can be generalized further. It's more obvious what it does. But I don't have a strong opinion on this.
tryIntegral
sounds reasonable for both Num n => Scientific -> Maybe n
and Integral i => i -> Maybe i
. For the Integral i
case toIntegral
also works.
just a note that from scientific 843186d1e8df72a3af4faa2ca4ed477c33066da5 on (will that be 0.4?), this function will have to call normalize or some lazier version of it in some cases.
just a note that from scientific 843186d on (will that be 0.4?),
I actually released this already as scientific-0.3.1.0.
this function will have to call normalize or some lazier version of it in some cases.
Why does this function have to normalize?
maybe i am misremembering things, but 100e-1
is an integer, but would be parsed as double. right?
maybe i am misremembering things, but 100e-1 is an integer, but would be parsed as double. right?
Oh good point! Yes, this function needs to normalize before checking if the number is an Integral or a RealFloat.
@bergmark, @ibotty what do you think about the following:
parseNumber :: (RealFloat r, Integral i) => Scientific -> Either r i
parseNumber s
| e < 0 = Left (toRealFloat s')
| otherwise = Right (fromInteger c * magnitude e)
where
s'@(Scientific c e) = normalize s
I choose to use RealFloat (Double, Float) instead of the more general Fractional because then I can use the toRealFloat function which has DoS protection.
The name question still stands ;-)
Looks good! I'll vote for naming it tryIntegral
any name is fine with me, but i'd like to retain the either behavior, because when i needed that function i always needed either that or that. and i think trySomething
returns a Maybe
so it would not fit that function signature imo.
regarding the implementation, shouldn't the following be more efficient?
parseNumber :: (RealFloat r, Integral i) => Scientific -> Either r i
parseNumber s@(Scientific _ e)
| e >= 0 = Right (toInteger s)
| otherwise = case (normalize s) of
s'@(Scientific _ e') -> if e' < 0 then Left (toRealFloat s')
else Right (toInteger s')
where
toInteger (Scientific c e) = fromInteger c * magnitude e
(of course it could certainly be structured more nicely.)
(writing code in that edit box sucks. i should have written as mail)
any name is fine with me, but i'd like to retain the either behavior, because when i needed that function i always needed either that or that. and i think trySomething returns a Maybe so it would not fit that function signature imo.
The reason I liked "try" before is that the signature resembles the signature of the try function. However, now that I think about it the 'Left' branch really represents failure which is not the case in our function.
So I agree, we should have a different name. What about eitherNumber
?
regarding the implementation, shouldn't the following be more efficient?
Yes, that would be more efficient. I refactored it a bit like:
-- | @eitherNumber s@ determines if the scientific is floating point
-- or integer. In case it's floating-point the scientific is converted
-- to the desired 'RealFloat' using 'toRealFloat'.
eitherNumber :: (RealFloat r, Integral i) => Scientific -> Either r i
eitherNumber s
| base10Exponent s >= 0 = Right (toIntegral s)
| base10Exponent s' >= 0 = Right (toIntegral s')
| otherwise = Left (toRealFloat s')
where
s' = normalize s
your version is much more clean of course...
So I agree, we should have a different name. What about |eitherNumber|?
everything is fine with me... but: eitherNumber sounds like a function
resembling either
. well. imo it's still the best name for now. note;
that i am pretty new to haskell, so my opinion regarding names should be
taken with a pot of salt ;).
Mmm I decided to go with the name:
floatingOrInteger
A bit long, but descriptive.
Adam, you have your function named floatingOrInteger
in scientific-0.3.2.0.
@bergmark, @ibotty thanks for your advice in this!
Thanks a bunch!
aeson 0.7 switched to scientific as the number reperesentation. I've run across a couple of packages that have code like this for aeson < 0.7:
Is there a recommended way to check for these cases using scientific? Some helper functions for this would be nice to have.
Cheers