iliekturtles / uom

Units of measurement -- type-safe zero-cost dimensional analysis
Apache License 2.0
1.01k stars 92 forks source link

feat: Add ton_per_minute and ton_per_day #405

Closed Uzaaft closed 1 year ago

Uzaaft commented 1 year ago

This PR attempts to add the ton_per_minute and ton_per_day units to flow rate. Regarding the name of ton, I'd like to propose changing it to tonne, to avoid confusion with long and short ton. Or metric_ton.

iliekturtles commented 1 year ago

Thanks for the PR! I'm inclined to keep ton as is for the time being until internationalize is implemented and we can have locale specific descriptions.

Below are a few changes. I think we might as well take this opportunity to move the comments into doc comments.

diff --git a/src/si/mass.rs b/src/si/mass.rs
index cd9a147..501f651 100644
--- a/src/si/mass.rs
+++ b/src/si/mass.rs
@@ -54,6 +54,7 @@ quantity! {
         @ton_assay: 2.916_667_E-2; "AT", "assay ton", "assay tons";
         @ton_long: 1.016_047_E3; "2240 lb", "long ton", "long tons";
         @ton_short: 9.071_847_E2; "2000 lb", "short ton", "short tons";
-        @ton: 1.0_E3; "t", "ton", "tons"; // ton, metric
+        /// Ton, metric.
+        @ton: 1.0_E3; "t", "ton", "tons";
     }
 }
diff --git a/src/si/mass_rate.rs b/src/si/mass_rate.rs
index 056bfa2..a21d220 100644
--- a/src/si/mass_rate.rs
+++ b/src/si/mass_rate.rs
@@ -95,12 +95,12 @@ quantity! {
             "short tons per second";
         @ton_short_per_hour: 2.519_957_5_E-1; "2000 lb/h", "short ton per hour",
             "short tons per hour";
-        @ton_per_second: 1.0_E3; "t/s", "ton per second",
-            "tons per second"; // ton per second, metric
-        @ton_per_minute: 1.666_666_666_666_666_6_E1; "t/min", "ton per minute",
-            "tons per minute"; // ton per minute, metric
-        @ton_per_day: 1.1574074074074E-2; "t/d", "ton per day",
-            "tons per day"; // ton per day, metric
+        /// Ton per second, metric.
+        @ton_per_second: 1.0_E3; "t/s", "ton per second", "tons per second";
+        /// Ton per minute, metric.
+        @ton_per_minute: 1.666_666_666_666_666_6_E1; "t/min", "ton per minute", "tons per minute";
+        /// Ton per day, metric.
+        @ton_per_day: 1.157_407_407_407_407_4_E-2; "t/d", "ton per day", "tons per day";
     }
 }
Uzaaft commented 1 year ago

Oh, I didn't know that docs after the function was a thing! I'll fix those right away

Uzaaft commented 1 year ago

Does the changes look Ok now?

Uzaaft commented 1 year ago

Adding ton_per_hour as well, forgot that one

iliekturtles commented 1 year ago

Changes look good. Kicked off the tests and if they pass I will merge.

iliekturtles commented 1 year ago

Actually, one last change! Can you squash your commits and update the commit message. Something like the following:

Add additional metric ton mass rate units.

`ton_per_minute`, `ton_per_hour`, `ton_per_day`.
Uzaaft commented 1 year ago

Attempted to squash them(I have no idea how to properly do that) 🤣

iliekturtles commented 1 year ago

Merged, thanks!