-
Notifications
You must be signed in to change notification settings - Fork 398
Added a few new units and new prefixes for existing units. #295
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
Conversation
…tishThermalUnitsInOneJoule
Sir @angularsen |
@angularsen sir, please see this article for reference https://portfoliomanager.zendesk.com/hc/en-us/articles/211025388-Is-there-a-list-of-valid-property-level-energy-meter-types-and-unit-of-measure-combinations- |
Ok, it's not that I don't trust you that someone is actually using such units, they do make sense, but my main concern is adding esoteric / rare units that no one else benefits from. I did google a bit more this time and I do find some references to usages of kilocubic feet, so I guess it's used enough in the wild to warrant being included in the library. Thanks for the swift creation of the new pull request, the changes look good! |
@@ -50,6 +54,8 @@ public class EnergyTests : EnergyTestsBase | |||
|
|||
protected override double FootPoundsInOneJoule => 0.737562149; | |||
|
|||
protected override double GigabritishThermalUnitsInOneJoule => 9.4781339449889105832843629746176e-13; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this one seems to still use the old constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops... my bad.
@@ -27,6 +27,8 @@ public class FlowTests : FlowTestsBase | |||
|
|||
protected override double CubicDecimetersPerMinuteInOneCubicMeterPerSecond => 60000.00000; | |||
|
|||
protected override double CubicFeetPerHourInOneCubicMeterPerSecond => 1.2713300937959091422759633856933e+5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is slightly different than CubicFeetPerSecond multiplied by 3600.
35.314666213 * 3600 = 1.271327983668e+5
Which one should be corrected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since CubicFeetPerSecond was already there, I would use 1.271327983668e+5
as a test value for CubicFeetPerHour
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed with two online converters cubic feet per second:
35.314666721489 ft3/s
from converter1
35.314 666 721 ft3/s
from converter2
The value is not perfectly identical, but pretty much good.
So, using the first value above times 3600, the per hour value should be: 127132.8001973604
.
If you could update both foot per second and foot per hour accordingly, then this should be good.
CubicFeetPerHourInOneCubicMeterPerSecond: 127132.8001973604
CubicFeetPerSecondInOneCubicMeterPerSecond: 35.314666721489
Found a couple more minor things, if you could address those then this should be ready to merge. |
Commenting here so it is visible, see comment above: #295 (comment) |
Made it. ;) |
Hooray :-) Thanks a bunch! Nuget 3.77 is now out. |
No description provided.