ahrefs / atd

Static types for JSON APIs
Other
308 stars 53 forks source link

Make it possible to round-trip the json `number` type #372

Closed rbjorklin closed 8 months ago

rbjorklin commented 8 months ago

I was asked to open a separate issue for this here.

I would like for there to be a way to round-trip numbers and have them come back with the same number of significant figures. I'm not familiar with the yojson code base but maybe Floatlit could be used to achieve this?

Example of the current behaviour: my_type.atd:

type my_type = {
    my_number : float;
    my_key : string;
}

my_type_test.ml:

let t = My_type_t.{ my_number = 65.26; my_key = "ABC" }

let () = 
   Printf.printf "%f" t.my_number;
    (* Prints: "65.260000" *)

    Printf.printf "%s" (My_type_j.string_of_my_type t)
    (* Prints: "{\"my_number\":65.26000000000001,\"my_key\":\"ABC\"}" *)

EDIT:

I'm currently using [Bigdecimal]() for my numeric functions and what I've got looks something like this:

type my_type = {
    my_number : float wrap <ocaml t="Bigdecimal.t" wrap="Bigdecimal.of_float_short_exn" unwrap="fun n -> Bigdecimal.round_to_power_of_ten ~power_of_ten:(-2) n |> Bigdecimal.to_float"> option;
    my_key : string;
}

If I could entirely skip using float as some kind of intermediate representation that would be good enough for me. I would also be okay parsing it into string as long as I can drop the quotes as I'm serializing it back to json.

mjambon commented 8 months ago

Preserving the original string representation of a float would require keeping it around alongside the float since in general there are many valid representations for the same float. Atdgen is focused on correctness i.e. it's the float→JSON→float roundtrip that matters, not JSON→float→JSON. So, it's unlikely that we can or will add a feature to support this specifically.

Yojson offers a Raw parsing mode and AST that keeps the original literals but I don't see how it could be used to solve this problem simply and conveniently with atdgen. Yojson.Raw would allow you to change the float literals into whatever literals you want or create a mapping from floats to preferred literals but it would make things slower and non-obvious. Is this feature something you really need, though?

rbjorklin commented 8 months ago

I'm interacting with a financial api endpoint which expects 2 decimals. It does seem to work sending floats but it is unclear to me what what actually happens on the other side. It's probably fine.

mjambon commented 8 months ago

If you know you need 2 decimals on all floats and you're worried that something like 1.999999 will be truncated to 1.99 instead of 2.00, it's a little simpler. You can postprocess the JSON produced by atdgen using Yojson.Raw: parse it, replace all Floatlit nodes, and print the JSON again.

If there are different float nodes with different precision requirements, you'll have to be careful but it's still doable. It's all very hacky, though. I wonder why this API doesn't use ints or maybe strings to represent decimal numbers.

rbjorklin commented 8 months ago

Given atd's focus on correctness of float -> JSON -> float as previously stated I think the conclusion here is that this works as intended and won't change. I'm okay with that as my wrapping into Bigdecimal.t with rounding on the unwrapping side should protect me from surprises.

Thanks again for your time @mjambon!

Closing!