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

RFC: Add Quantity::as_ and Quantity::cast for changing the underlying storage type. #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamreichold
Copy link
Contributor

@adamreichold adamreichold commented Mar 25, 2022

A first go at something helping with #293.

The restriction to $units<V> instead of U: Units<V> seems a bit weird, but it was the only way I saw to enforce that the same set of base units would be used for the conversion. (The more general version fails with trait bound errors as e.g. the f32 base units do not implement Units<i32> and without generic associated types there is no way to ask a Units<V> implementor for its equivalent Units<W> implementor AFAIU.)

Finally, while I added minimal doc tests, I am not sure where actual unit tests should end up as this involves at least two storage types by definition.

@adamreichold
Copy link
Contributor Author

adamreichold commented Mar 26, 2022

without generic associated types there is no way to ask a Units implementor for its equivalent Units implementor AFAIU

I think we could add a separate trait

trait ChangeStorage<Ul, Ur>: Units<Ul> {
    type Units: Unit<Ur>;
}

impl<Ul, Ur> ChangeStorage<Ul, Ur> for $units<Ul> {
    type Units = $units<Ur>;
}

but I am not sure how much this would add w.r.t. generality as it would still cover only the $units<_> case by default.

@iliekturtles
Copy link
Owner

Thanks for the PR! I'll take a look this coming week and see if I can figure out anything with the traits.

@iliekturtles
Copy link
Owner

I don't think using $units is needed/desired.

The following diff compiles so it works... right?

I went a slightly different way and defined a QuantityCast trait although I suppose no trait or even From/Into (too implicit?) could be used. This is a very rough implementation and essentially delegates the conversion down to num::cast::NumCast which in turn relies on num::cast::ToPrimitive.

I'll do some thinking about usability over the weekend as I think the ability to do type conversions is proven, it's just a matter of determining what the interface should be.

diff --git a/src/lib.rs b/src/lib.rs
index be430b4..32b73cf 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -280,6 +280,10 @@ pub mod num {
     #[cfg(feature = "bigint-support")]
     pub use num_rational::BigRational;
 
+    pub mod cast {
+        pub use num_traits::cast::*;
+    }
+
     #[cfg(any(feature = "rational-support", feature = "bigint-support"))]
     pub mod rational {
         pub use num_rational::*;
diff --git a/src/system.rs b/src/system.rs
index 6324553..e9aac7c 100644
--- a/src/system.rs
+++ b/src/system.rs
@@ -590,6 +590,34 @@ macro_rules! system {
         impl_ops!(Sub, sub, -, SubAssign, sub_assign, -=, Diff,
             Div, div, /, DivAssign, div_assign, /=, sub_div);
 
+        trait QuantityCast<D, U, V>: Sized
+        where
+            D: Dimension + ?Sized,
+            U: Units<V> + ?Sized,
+            V: $crate::num::Num + $crate::num::cast::ToPrimitive + $crate::Conversion<V>,
+        {
+            fn from(q: Quantity<D, U, V>) -> Option<Self>;
+        }
+
+        impl<D, Ur, Ul, Vr, Vl> QuantityCast<D, Ul, Vl> for Quantity<D, Ur, Vr>
+        where
+            D: Dimension + ?Sized,
+            Ur: Units<Vr> + ?Sized,
+            Ul: Units<Vl> + Units<Vr> + ?Sized,
+            Vr: $crate::num::Num + $crate::num::cast::NumCast + $crate::Conversion<Vr>,
+            Vl: $crate::num::Num + $crate::num::cast::ToPrimitive + $crate::Conversion<Vl>,
+        {
+            fn from(q: Quantity<D, Ul, Vl>) -> Option<Self>
+            {
+                <Vr as $crate::num::cast::NumCast>::from(q.value)
+                    .map(|v| Quantity {
+                        dimension: $crate::lib::marker::PhantomData,
+                        units: $crate::lib::marker::PhantomData,
+                        value: change_base::<D, Ul, Ur, Vr>(&v),
+                    })
+            }
+        }
+
         impl<D, U, V> Quantity<D, U, V>
         where
             D: Dimension + ?Sized,

@adamreichold
Copy link
Contributor Author

The trait bound which did not work out for me was

Ul: Units<Vl> + Units<Vr> + ?Sized,

as e.g. SI<f32> would only implement Units<f32> but not say Units<i32>.

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