Conversion from primitives not working with integer values about dimensioned HOT 9 CLOSED

The problem is that PX is a constant with type Pixel<f64>. The macro defining a unit system only creates constants over f32 and f64.

You have a couple options. One is to not use the constants, and set x equal to Pixel::new(42).

Another is to define them yourself, which would look like

const PX: Pixel<isize> = Pixel { value_unsafe: 1, _marker: PhantomData };

Finally, you can submit an issue or pull request to add integer constants to the macro.

It shouldn't be too much work if you want to do it. I think it's just a matter of adding i32consts, i64consts, ... similarly to the float versions in make_units.rs, and then adding some integer tests to make sure they work, and updating the make_units doc to include them.

I'm pretty busy right now, but if you'd like me to do it, go ahead and submit an issue to have them added and I'll hopefully get to it in the next few days.

from dimensioned.

A note: I know it's just a tiny test, so feel free to ignore this.

I would encourage you to avoid uning value_unsafe where possible. For example, your assert could be changed to

assert_eq!((x/PX).value(), 42.0);

or

assert_eq!(*(x/PX), 42.0);

Which makes the units of x clear, and will fail to compile if you're wrong about them.

from dimensioned.

Thanks for the prompt reply, and thanks for the notes re: value_unsafe. The test was just to show a complete example rather than real code, but it's good to know the right way.

from dimensioned.

I've had a quick peek at adding the i32/i64 constants, and I think it's beyond me.

Actually getting the macros to generate the basic conversions was pretty straightforward, and I managed to build a cut down SI prefix list which seemed to work.

The complication comes when you start having to consider all the user-defined constants in a system. Once you start generating integer constants you start truncating these user-supplied values and suddenly they don't make sense any more.

I'm not sure what the correct approach is at them moment. Casting the values to i32/i64 is easy enough - it just feels like it would be surprising behavour for someone using the library.

from dimensioned.

I'll take a look at it when I have time. I had a thought ... It might work to define constants with a unit struct for the value type, such that you can multiply them by anything to give units. I think it would require specialization to work properly, but I'll have to play with it to be sure. It also would not work for user defined constants, just the base and derived ones.

from dimensioned.

I'm imagining something like this (grossly simplified for clarity):

use std::ops::Mul;

#[derive(Debug)]
struct Value<T> {
    v: T,
}

struct PX;

impl Mul<PX> for i32 {
    type Output = Value<i32>;
    fn mul(self, _: PX) -> Self::Output {
        Value::<i32> { v: self }
    }
}

impl Mul<PX> for f64 {
    type Output = Value<f64>;
    fn mul(self, _: PX) -> Self::Output {
        Value::<f64> { v: self }
    }
}

fn main() {
    let x = 42 * PX;
    let y = 42.3 * PX;
    println!("X = {:?}, y = {:?}", x, y);
}

Is that the sort of idea?

from dimensioned.

My idea was something like

struct Constant;

and then in the make_units macro

impl<T, U> Mul<T> for $system<Constant, U> {
    type Output = $system<T, U>;
    fn mul(self, rhs: T) -> Self::Output {
        $system::new(rhs)
    }
}

and also

impl<U> Mul<$system<Constant, U>> for $t {
    type Output = $system<$t, U>;
    fn mul(self, _: $system<Constant, U>) -> Self::Output {
        $system::new($t)
    }
}

where we have such a block for $t as each primitive.

Then if you have,

const PX: Pixel<Constant> = Pixel { value_unsafe: Constant, _marker: PhantomData };

you can do let x: Pixel<f64> = 3.0 * PX; and let y: Pixel<i32> = 3 * PX;

The problem comes when you try to do anything more involved than constants...

Wait, might be able to just do

impl<T> Mul<T> for Constant {
    type Output = T;
    fn mul(self, rhs: T) -> T {
        rhs
    }
}

and then

impl Mul<Constant> for $t {
    type Output = $t;
    fn mul(self, _: Constant) -> $t {
        self
    }
}

for each primitive as $t....

That might give as much functionality as we already have. I'll have to play with it to see.

from dimensioned.

Sorry it's taken a while, but I've finally had a few minutes to sit down and look at this.

I've added modules i8consts, i16consts, i32consts, i64consts, and isize_consts (and the same for unsigned ints) to the make_units! macro. Right now, they don't include anything from the constants block, just base and derived. I think this is the best solution, along with adding an optional int_consts block to the macro for defining more constants which I'll get to at some point.

These changes exist in the github repo, but have not been published on crates.io yet. I would like to do more testing and add documentation before doing that, as well as adding that int_consts block, but I really don't have much time right now, so it may be a little while.

Anyway, if you depend on the github version and switch pub use self::f64consts::*; to pub use self::isize_consts::*; in your make_units! invocation, it should behave as you want it to. Please let me know if it doesn't.

from dimensioned.

As of version 0.7.0, integer constant modules are generated by make_units! with all base and derived units. Also, an admittedly non-ideal way of constructing more constants is mentioned in the docs:

https://docs.rs/dimensioned/0.7.0/dimensioned/macro.make_units.html

Please feel free to re-open this if that does not suit your needs.

from dimensioned.