ta-rs
ta-rs copied to clipboard
feat: Support for Decimal within ta-rs.
Okay @greyblake, here's how I have in mind approaching adding Decimal
support for those that want it.
Essentially the idea is to add an optional dependency on rust_decimal
, and swap the type based on whether or not the feature is added. I added this:
#[cfg(not(feature = "rust_decimal"))]
pub(crate) type NumberType = f64;
#[cfg(feature = "rust_decimal")]
pub(crate) type NumberType = rust_decimal::Decimal;
And then it's mostly a matter of taking the f64
s in the various indicators and changing them to NumberType
.
I'll probably need to do some work to get the tests working in both cases, since you (quite reasonably) use literals in a lot of tests. I'm not sure off hand how many types rust_decimal
implements Into<Decimal>
for, but worst case I can write a macro to make the tests do the right thing without a lot of repetition.
Fixes #55.
Note: I'm by no means done, I wanted to get feedback from you before doing the grunt work. :-)
Okay, I've gotten farther on this. The concept works and seems to scale. I've converted all the indicators and am about 2/3 of the way through the tests.
The major downside is literals in tests. Integers generally do the right thing but floats don't, so I had to write a lit!
(literal) macro and use it on literals. This adds a trivial tax on writing indicators (there are about 7-8 literals total) but a noticeable tax on writing tests, which use literals liberally. I still think this is a worthwhile endeavor and hope you'll accept it, but wanted to be transparent.
Okay @greyblake, this is done (with one caveat) and ready for review. I've detailed the overall approach above.
One thing that's not done is the benchmark, which requires rust_decimal
to support rand
. As it happens, they've just added that (three days ago; I did not do it), so ~I'm just waiting for a 1.24 release (which I have also politely inquired about).~
EDIT: It turns out the rand implementation was partial; I've sent paupino/rust-decimal#519 to provide what the benchmark needs.
- Tests all pass both with and without the
decimal
feature. - Examples only pass without
decimal
, and I've configured CI accordingly. It did not seem proper to me to clutter up your documentation / examples.
@greyblake This is now ready! :-)
@greyblake Ping! :-)
@lukesneeringer Pong. Sorry for delays, I barely have time these days because of personal reasons.