-
Notifications
You must be signed in to change notification settings - Fork 100
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
added AngularVelocity #135
Conversation
Thanks for these PRs! I'm going to try hard to a thorough review this weekend as I'm going to have limited internet access starting next week through Memorial day. |
src/si/angle.rs | 3 +--
src/si/angular_velocity.rs | 18 +++---------------
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/src/si/angle.rs b/src/si/angle.rs
index 4d82719..7830bd0 100644
--- a/src/si/angle.rs
+++ b/src/si/angle.rs
@@ -62,8 +62,7 @@ mod convert {
mod tests {
storage_types! {
use ::lib::f64::consts::PI;
- use num::One;
- use num_traits::FromPrimitive;
+ use num::{FromPrimitive, One};
use si::angle as a;
use si::quantities::*;
use tests::Test;
diff --git a/src/si/angular_velocity.rs b/src/si/angular_velocity.rs
index 703bb10..f1cf13e 100644
--- a/src/si/angular_velocity.rs
+++ b/src/si/angular_velocity.rs
@@ -1,9 +1,9 @@
-//! Angular Velocity (base unit radian per second, s⁻¹).
+//! Angular velocity (base unit radian per second, s⁻¹).
quantity! {
/// Angular velocity (base unit radian per second, s⁻¹).
quantity: AngularVelocity; "angular velocity";
- /// Dimension of angular velocity (base unit radian per second, s⁻¹).
+ /// Dimension of angular velocity, T⁻¹ (base unit radian per second, s⁻¹).
dimension: ISQ<
Z0, // length
Z0, // mass
@@ -31,8 +31,7 @@ quantity! {
mod tests {
storage_types! {
use ::lib::f64::consts::PI;
- use num::One;
- use num_traits::FromPrimitive;
+ use num::{FromPrimitive, One};
use si::angle as a;
use si::angular_velocity as v;
use si::quantities::*;
@@ -41,17 +40,6 @@ mod tests {
#[test]
fn check_units() {
- Test::assert_eq(&AngularVelocity::new::<v::radian_per_second>(V::from_f64(2.0 * PI).unwrap()),
- &AngularVelocity::new::<v::degree_per_second>(V::from_f64(360.0).unwrap()));
- Test::assert_eq(&AngularVelocity::new::<v::revolution_per_second>(V::one()),
- &AngularVelocity::new::<v::degree_per_second>(V::from_f64(360.0).unwrap()));
- Test::assert_eq(&AngularVelocity::new::<v::revolution_per_second>(V::one()),
- &AngularVelocity::new::<v::radian_per_second>(V::from_f64(2.0 * PI).unwrap()));
- Test::assert_eq(&AngularVelocity::new::<v::revolution_per_second>(V::one()),
- &AngularVelocity::new::<v::revolution_per_minute>(V::from_f64(60.0).unwrap()));
- Test::assert_eq(&AngularVelocity::new::<v::revolution_per_second>(V::one()),
- &AngularVelocity::new::<v::revolution_per_hour>(V::from_f64(3600.0).unwrap()));
-
test::<a::radian, t::second, v::radian_per_second>();
test::<a::degree, t::second, v::degree_per_second>();
test::<a::revolution, t::second, v::revolution_per_second>(); |
I would rather keep the assert_eqs because the code that they're exercising (the constants) is not covered by the Angle tests. That is, it's easy to selectively break one set of tests but not the other. I've previously run into a particular type of bug enough times that I really watch for it now. It stems from the fact that Ctrl+T opens a new tab in a browser, but in editors it transposes characters, and when I'm lost in some other thing hotkeys happen subconsciously. If the cursor happens to be in a string or numeric literal when that happens it's basically impossible to spot (without tests). Basically I'd rather leave those tests to protect me from myself. Edit: check that, I didn't fully understand what you were saying. You're right, I'll take them out. |
Manually merged again. Thanks for the PR! |
No description provided.