cmpute / dashu

A library set of arbitrary precision numbers implemented in Rust.
Apache License 2.0
74 stars 9 forks source link

Support `Serialize` + `Deserialize` for `dashu-ratio` #20

Closed oovm closed 1 year ago

oovm commented 1 year ago
"-5/11"
{
    "sign": true,
    "numerator": [5],
    "denominator": [11]
}
oovm commented 1 year ago

Well, sorry, seems not work with no-std

cmpute commented 1 year ago

Thanks for your contribution, but the implementation seems over-complicate for me. Do you have any specific reason to serialize the rational number as a struct with three parts? I was planning to serialize the rational number just as a pair of integers.

Besides, you can take a look at #11, where I listed all thirdparty crates I planned to support, so you don't have to implement them yourselves.

oovm commented 1 year ago

How about dividing into the following three binary representations?

  1. [ ] => 0 / 1
  2. [i64, bytes<numerator>] => numerator / 1
  3. [i64, bytes<numerator>, u64, bytes<denominator>] => numerator / denominator
oovm commented 1 year ago

Maybe VLQ is better than i64 and u64, since it also supports non-64-bit platforms.

It is best to introduce the byteorder library, byteorder defines a set of convenient binary reading interfaces.

cmpute commented 1 year ago

I don't think we need to handle the VLQ implementation ourselves, it could be handled by serializers like the postcard. I already have serde support for UBig and IBig, we should reuse the implementation of that. (Although I am planning to refactor it to use byte array instead of the word array which is currently used).

What I have planned for the (binary) serialization formats is:

For text serialization, I would just use the format result of each type.

cmpute commented 1 year ago

serde support for dashu-float and dashu-ratio has been added in https://github.com/cmpute/dashu/commit/d184f7cbb7654ccad692d7ff14d88c8e9f333e67, the improvements on serializing UBig and IBig will be added after releasing the next minor version, because that is a break change.

Thanks for your help but I will close this issue for now.