ianmackenzie / elm-units

Simple, safe and convenient unit types and conversions for Elm
https://package.elm-lang.org/packages/ianmackenzie/elm-units/latest/
BSD 3-Clause "New" or "Revised" License
85 stars 14 forks source link

Add basic units/functions for VolumetricFlow #43

Closed jamessral closed 2 years ago

jamessral commented 5 years ago
ianmackenzie commented 5 years ago

Looks pretty good! Could you try to modify the introductory paragraph to follow a pattern like https://package.elm-lang.org/packages/ianmackenzie/elm-units/latest/Current? (I like to point out that rates like volumetric flow can be expressed using Quantity.per.)

Looking at https://en.wikipedia.org/wiki/Volumetric_flow_rate, I think it would be useful to also add

cubicCentimetersPerMinute
cubicFeetPerSecond
imperialGallonsPerMinute
usLiquidGallonsPerMinute
usDryGallonsPerMinute

as well as the corresponding in functions.

Finally, it would also be great to add a few tests to tests/Tests.elm - add a volumetricFlows test similar to volumes, and a "VolumetricFlow" section to the big conversionsToQuantityAndBack test.

jamessral commented 5 years ago

Sounds good. Will do

ianmackenzie commented 5 years ago

Hi James, just had a look and made a few small changes:

I also added a skeleton volumetricFlows test which found a couple errors in conversions such as the various "gallons per minute" functions - one thing I recently started doing (in a PR for Molarity that just got merged!) was adding helper constants to the top of the file and then using those within the various conversion functions.

I've done that for US liquid gallons per minute so you can see the pattern; can you update the other tricky conversion functions to use a similar pattern, and add corresponding cases to the volumetricFlows test? It would also be great to have a case in the general conversionsToQuantityAndBack for every volume unit to make sure they're all working.