add i64 and f32 numeric types

[lambertsbennett]

I really like charming for plotting, but I ran into some problems trying to plot i64 values. In this PR I just added the possibility to use f32 and i64 values. If there is a good reason why this currently isn't supported let me know!

[yuankunzhang]

Hi @lambertsbennett , thanks for the pull request. The rational behind not having f32 here is that the underling echarts.js only has double precision float numbers; and the use of i32 here was a decision influenced by my limited foresight 😅. It makes sense to convert the i32 in the enum to i64, as below:

pub enum DataSource {
    Integers(Vec<Vec<i64>>),
    Floats(Vec<Vec<f64>>),
    Mixed(Vec<Vec<CompositeValue>>),
}

[lambertsbennett]

Ah ok! I figured it might have something to do with echarts. Does it also need to be changed here?

pub enum NumericValue {
    Integer(i64),
    Float(f64),
}

I can strip the other stuff away and test it out first thing tomorrow!

[yuankunzhang]

That would be great 👍

[lambertsbennett]

@yuankunzhang busy day at work today, but I hope to get to this this week. I guess there is a bit of work that needs to be done to implement conversion traits for i32 -> i64 and f32 -> f64.

[lambertsbennett]

@yuankunzhang ok so I added logic to deal with f32 -> f64 and i32 -> i64 conversions. One thing that I am not pleased with right now is the conversion from f32 -> f64 leads to some loss of precision. This is probably not crucial, but it could be problematic in some cases. I also had to change one of the tests as follows:

    #[test]
    #[should_panic]
    fn numeric_value_from_f32() {
        let n: NumericValue = 0.618f32.into();
        assert_eq!(n, NumericValue::Float(0.618));
    }

Because the f32.into yields something like 0.6179999999. If people are using f64 values out of the box there are no problems, but I worry about silent loss of precision if people are using f32 values. I don't know if I'm making a mountain out of a mole hill though.

[yuankunzhang]

@lambertsbennett thanks for the PR! It looks good to me. The minor precision loss shouldn't be a concern for chart rendering.

[yuankunzhang]

LGTM