Skip to content
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

encodeFixed? #5

Open
mwotton opened this issue Feb 6, 2023 · 2 comments
Open

encodeFixed? #5

mwotton opened this issue Feb 6, 2023 · 2 comments

Comments

@mwotton
Copy link

mwotton commented Feb 6, 2023

Would you accept a patch that adds a function that always encodes in a fixed decimal format? I'm using your code to process some JSON into CSV and would prefer a simple & predictable numeric encoding format, even if it's pretty bloaty.

(Thanks for the cool software, btw, json-syntax is way faster than Aeson for my workload.)

@andrewthad
Copy link
Member

I believe that you're talking about a function that looks like this:

encodeFixed ::
     Int -- ^ Number of digits to include after the period
  -> Scientific -- ^ Number to encode
  -> ShortText

If we do something like:

encodeFixed 3 (small 5 0)

Would the output be 5 or 5.000? I think it would be the second one, but it would be good to be precise in the function's documentation. And I think it would have truncating (rather than rounding) behavior for numbers that could not be represented exactly. Alternatively, you might want a function that's more like this:

encodeDecimal ::
     Scientific -- ^ Number to encode
  -> ShortText

Which just always does the maximum number of decimal places. This second one is actually pretty easy to write since builderUtf8 currently has a special case for exponents between -50 and +50:

builderUtf8 :: Scientific -> Builder
builderUtf8 (Scientific coeff e big)
  | e == 0 = Builder.intDec coeff
  | e == minBound = let LargeScientific coeff' e' = big in
      if | coeff' == 0 -> Builder.ascii '0'
         | e' == 0 -> Builder.integerDec coeff'
         | e' > 0 && e' < 50 ->
             -- TODO: Add a replicate function to builder to improve this.
             Builder.integerDec coeff' <> Builder.bytes (Bytes.replicate (fromInteger e') 0x30)
         | e' < 0, e' > (-50), coeff' > 0, coeff' < 18446744073709551616 ->
             let coeff'' = fromInteger coeff' :: Word
                 e'' = fromInteger e' :: Int
              in Builder.bytes (encodePosCoeffNegExp coeff'' e'')
  ...

So to implement encodeDecimal, all that is needed is to copy builderUtf8 and make the [-50,+50] "special case" the only code path.

I'd take either of these in a PR as long as some relevant tests are added to the test suite.

@mwotton
Copy link
Author

mwotton commented Feb 10, 2023

thanks! I think the second is closer to what I want, I'll get a PR together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants