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

Adds serde_derive feature #2

Closed
wants to merge 1 commit into from
Closed

Adds serde_derive feature #2

wants to merge 1 commit into from

Conversation

MathisWellmann
Copy link
Contributor

@MathisWellmann MathisWellmann commented Mar 6, 2023

to allow serialization and deserialization of the Decimal struct.
It only takes effect if the dependency is specified as such:

fpdec = { version = "*", features = ["serde_derive"] }

The reason for this addition is that I would like to be able to rely on serde support in lfest to serialize the BaseCurrency and QuoteCurrency among others.
I would love to see this change get merged and published so a new version of lfest can be published as well.

Thank you for such an amazing crate, I'm glad I found it!

@mamrhein
Copy link
Owner

mamrhein commented Mar 6, 2023

Thx for the PR.
Unfortunately, Decimal is using an i128 integer for the coefficient and serde doesn't seem to support integers larger than 64-bit out of the box.
I'll have to dig into serde to find a solution.

@MathisWellmann
Copy link
Contributor Author

After some re-thinking, I will not directly serialize or deserialize the Decimal type as its rather confusing from a user-facing perspective to have to specify the inner fields. Consider this PR low priority or feel free to close it as well.
Cheers!

@mamrhein
Copy link
Owner

mamrhein commented Mar 7, 2023

I've created a branch 'serde'.
The feature 'serde_as_str' allows Decimal instances to be serialized to / deserialized from String.
Maybe you want to give it a try.
If so, please let me know what you think about it.

@MathisWellmann
Copy link
Contributor Author

Just tried out your new feature branch and it looks excellent.
Using the From<Decimal> and TryFrom<String> trait implementations through the serde features is a fantastic way, especially from the users perspective (As they should not be bothered to specify the coeff).
Having to enable both serde and serde_as_str features is the right choice as well, as not everyone will want to use String as the intermediate representation.
Integrating the feature is straightforward as well.
I certainly would be happy seeing these changes in a crates.io release.
Gute Arbeit :)

@mamrhein
Copy link
Owner

Solved by adding feature 'serde-as-str'. See version 0.6.0.

@mamrhein mamrhein closed this Mar 14, 2023
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

Successfully merging this pull request may close these issues.

2 participants