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

Add support for solid angles mostly based on what we do for angles. #196

Merged
merged 2 commits into from
Jul 21, 2020
Merged

Add support for solid angles mostly based on what we do for angles. #196

merged 2 commits into from
Jul 21, 2020

Conversation

adamreichold
Copy link
Contributor

Closes #190

src/si/solid_angle.rs Outdated Show resolved Hide resolved
@iliekturtles
Copy link
Owner

Thanks for the PR! I'll do a code review today!

@adamreichold adamreichold changed the title A support for solid angles mostly based on what we do for angles. Add support for solid angles mostly based on what we do for angles. Jul 20, 2020
Copy link
Owner

@iliekturtles iliekturtles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good! I only found a couple formatting issues. If you can amend these into your commit that adds SolidAngle I'll merge!

diff --git a/src/si/mod.rs b/src/si/mod.rs
index eba8ae4..3700cf4 100644
--- a/src/si/mod.rs
+++ b/src/si/mod.rs
@@ -118,7 +118,7 @@ storage_types! {
 pub mod marker {
     use si::{Dimension, Quantity, Units};
     use Kind;
-
+    
     /// `AngleKind` is a `Kind` for separating angular quantities from their identically dimensioned
     /// non-angular quantity counterparts. Conversions to and from `AngleKind` quantities are
     /// supported through implementations of the `From` trait.
@@ -133,8 +133,8 @@ pub mod marker {
     pub trait AngleKind: ::Kind {}
 
     /// `SolidAngleKind` is a `Kind` for separating quantities of solid angles from other
-    /// identically dimensioned quantities. Conversions to and from `SolidAngleKind` quantities
-    /// are supported through implementations of the `From` trait.
+    /// identically dimensioned quantities. Conversions to and from `SolidAngleKind` quantities are
+    /// supported through implementations of the `From` trait.
     ///
     #[cfg_attr(feature = "f32", doc = " ```rust")]
     #[cfg_attr(not(feature = "f32"), doc = " ```rust,ignore")]
@@ -145,9 +145,9 @@ pub mod marker {
     /// ```
     pub trait SolidAngleKind: ::Kind {}
 
-    /// InformationKind is a `Kind` for separating information quantities from their identically
-    /// dimensioned non-information quantity counterparts. Conversions to and from
-    /// `InformationKind` quantities are supported through implementations of the `From` trait.
+    /// `InformationKind` is a `Kind` for separating information quantities from their identically
+    /// dimensioned non-information quantity counterparts. Conversions to and from `InformationKind`
+    /// quantities are supported through implementations of the `From` trait.
     ///
     #[cfg_attr(feature = "f32", doc = " ```rust")]
     #[cfg_attr(not(feature = "f32"), doc = " ```rust,ignore")]
diff --git a/src/si/solid_angle.rs b/src/si/solid_angle.rs
index b134441..95773f2 100644
--- a/src/si/solid_angle.rs
+++ b/src/si/solid_angle.rs
@@ -14,8 +14,8 @@ quantity! {
         Z0>;    // luminous intensity
     kind: dyn (::si::marker::SolidAngleKind);
     units {
-        /// SI derived unit of solid angle is steradians. It is the solid angle
-        /// subtended at the center of a unit sphere by a unit area on its surface.
+        /// SI derived unit of solid angle is steradians. It is the solid angle subtended at the
+        /// center of a unit sphere by a unit area on its surface.
         @steradian: 1.0_E0; "sr", "steradian", "steradians";
         @spat: 1.256_637_061_435_917_3_E1; "sp", "spat", "spats";
         @square_degree: 3.046_174_197_867_086_E-4; "°²", "square degree", "square degrees";
@@ -26,7 +26,8 @@ quantity! {
 
 #[cfg(feature = "f32")]
 impl SolidAngle<::si::SI<f32>, f32> {
-    /// The solid angle subtended by a sphere at its center, i.e. with a value 4π as measured in steradians
+    /// The solid angle subtended by a sphere at its center, i.e. with a value 4π as measured in
+    /// steradians.
     pub const SPHERE: Self = Self {
         dimension: ::lib::marker::PhantomData,
         units: ::lib::marker::PhantomData,
@@ -36,7 +37,8 @@ impl SolidAngle<::si::SI<f32>, f32> {
 
 #[cfg(feature = "f64")]
 impl SolidAngle<::si::SI<f64>, f64> {
-    /// The solid angle subtended by a sphere at its center, i.e. with a value 4π as measured in steradians
+    /// The solid angle subtended by a sphere at its center, i.e. with a value 4π as measured in
+    /// steradians.
     pub const SPHERE: Self = Self {
         dimension: ::lib::marker::PhantomData,
         units: ::lib::marker::PhantomData,
@@ -97,11 +99,15 @@ mod tests {
         fn check_units() {
             Test::assert_eq(&SolidAngle::new::<sa::steradian>(V::from_f64(4.0 * PI).unwrap()),
                 &SolidAngle::new::<sa::spat>(V::one()));
-            Test::assert_approx_eq(&SolidAngle::new::<sa::square_degree>(V::from_f64(360.0 * 360.0 / PI).unwrap()),
+            Test::assert_eq(
+                &SolidAngle::new::<sa::square_degree>(V::from_f64(360.0 * 360.0 / PI).unwrap()),
                 &SolidAngle::new::<sa::spat>(V::one()));
-            Test::assert_approx_eq(&SolidAngle::new::<sa::square_minute>(V::from_f64(60.0 * 60.0).unwrap()),
+            Test::assert_eq(
+                &SolidAngle::new::<sa::square_minute>(V::from_f64(60.0 * 60.0).unwrap()),
                 &SolidAngle::new::<sa::square_degree>(V::one()));
-            Test::assert_approx_eq(&SolidAngle::new::<sa::square_second>(V::from_f64((60.0 * 60.0) * (60.0 * 60.0)).unwrap()),
+            Test::assert_eq(
+                &SolidAngle::new::<sa::square_second>(
+                    V::from_f64((60.0 * 60.0) * (60.0 * 60.0)).unwrap()),
                 &SolidAngle::new::<sa::square_degree>(V::one()));
         }
     }

src/si/solid_angle.rs Outdated Show resolved Hide resolved
@adamreichold
Copy link
Contributor Author

@iliekturtles Applied the patch. Could you give me a hint on how you keep the formatting in check as rustfmt will not touch the stuff inside the macros reliably? (I have admittedly become dependent on rustfmt and hardly even consider formatting while writing code anymore.)

@iliekturtles
Copy link
Owner

Build failed which is my fault! I was testing assert_approx_eq vs assert_eq and forgot to remove that before posting the patch. If you can switch the last three back to assert_approx_eq everything should work then I can merge for real.

For formatting I just spend a bunch of time looking at the code. I have vertical guidelines setup at the 100 character mark to see when text overflows and VIM's gq operator mostly works to re-wrap comments.

@adamreichold
Copy link
Contributor Author

Switched back to assert_approx_eq.

@iliekturtles iliekturtles merged commit 5dca5a6 into iliekturtles:master Jul 21, 2020
@iliekturtles
Copy link
Owner

Thanks again for the PR. Merged!

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.

Add support for solid angles and steradian
2 participants