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

added new units to the TemperatureChangeRate quantity #1433

Merged

Conversation

David-HH-Dev
Copy link
Contributor

@David-HH-Dev David-HH-Dev commented Nov 11, 2024

added the following units to TemperatureChangeRate:
DegreeFahrenheitPerMinute
DegreeFahrenheitPerSecond
DegreeKelvinPerSecond
DegreeCelsiusPerHour
DegreeKelvinPerHour
DegreeFahrenheitPerHour

After reviewing the unit tests I came to the conclusion that the recently added (#1429) unit DegreeKelvinPerMinute had a wrong conversion.

I checked that for example here:
https://www.unitsconverters.com/en/Rate-Of-Temperature-Change-Conversions/Measurement-1189
and also here:
https://www.quora.com/Why-is-32-not-used-when-converting-per-degree-Celsius-C-1-to-per-degree-Fahrenheit-F-1

"FromUnitToBaseFunc": "({x} / 60) - 273.15",
"FromBaseToUnitFunc": "({x} + 273.15) * 60",
"FromUnitToBaseFunc": "{x} / 60",
"FromBaseToUnitFunc": "{x} * 60",
Copy link
Owner

Choose a reason for hiding this comment

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

Good catch, I missed this when I last reviewed this by checking google, which gives the wrong answer:
https://www.google.com/search?q=DegreesKelvinPerMinute+In+1+DegreeCelsiusPerSecond&rlz=1C1ONGR_enNO1008NO1008&oq=DegreesKelvinPerMinute+In+1+DegreeCelsiusPerSecond&gs_lcrp=EgZjaHJvbWUqBggAEEUYOzIGCAAQRRg70gEINTE2NmowajeoAgCwAgA&sourceid=chrome&ie=UTF-8

It totally makes sense for kelvin and celsius rate of change to be equal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@angularsen thanks for merging my PR. Do you know (roughly) when you will release a new Nuget package?

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the reminder, I got busy and forgot it (again). Maybe it is my age 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

ahhh thank you for catching my mistake, as well as the links explaining the correct calculation!!

@angularsen angularsen merged commit 1d1d5ac into angularsen:master Dec 7, 2024
1 check was pending
@angularsen
Copy link
Owner

Thanks! Nuget should be out shortly.

Release UnitsNet/5.61.0 · angularsen/UnitsNet

@David-HH-Dev
Copy link
Contributor Author

Thanks! Nuget should be out shortly.

Release UnitsNet/5.61.0 · angularsen/UnitsNet

Thank you @angularsen
Sorry for bothering you again, but would you know of a reason why the release isn't available through Nuget yet?

@angularsen
Copy link
Owner

@David-HH-Dev Thanks for checking in, turns out there was a build error due to some new vulnerability in a Nuget.Protocol nuget package (meta!)

I just pushed a fix, let's see if the nuget gets deployed in the next 30 minutes or so.
https://dev.azure.com/unitsnet/Units.NET/_build/results?buildId=932&view=results

@angularsen
Copy link
Owner

Another brand new compile error 🙄
Second attempt
https://dev.azure.com/unitsnet/Units.NET/_build/results?buildId=936&view=results

@angularsen angularsen added this to the v6 milestone Dec 29, 2024
@angularsen
Copy link
Owner

Merged into V6 branch, nuget should be out shortly.

Release UnitsNet/6.0.0-pre013 · angularsen/UnitsNet

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.

3 participants