search

angularsen / UnitsNet

Makes life working with units of measurement just a little bit better.
https://www.nuget.org/packages/UnitsNet/
MIT No Attribution
2.63k stars 383 forks source link

Temperature Gradient #988

Closed dglozano closed 2 years ago

dglozano commented 3 years ago

Hi 👋🏻 I am using UnitsNet quite a lot and I really love it! So far it has covered most of my use cases, but I recently I got the need to use a TemperatureGradient quantity (https://en.wikipedia.org/wiki/Temperature_gradient).

I know there is LapseRate quantity which is also TemperatureDelta / Length, but it seems to be "semantically" focused on the atmospheric temperature variation and it supports only the DegreesPerKilometer unit.

I was wondering if adding a new TemperatureGradient is an option? If so, I would be willing to create a PR to add it.

Another option would be to make the LapseRate support more units I guess, but both the name and description of the quantity can be a little misleading.

Related (ish) to #324

angularsen commented 3 years ago

Hi, without knowing this domain I would favor moving everything to TemperatureGradient. It might be more consistent with our other quantities to name it TemperaturePerLength, but it seems TemperatureGradient is a well established quantity and maybe the better choice .

LapseRate has been touched in:

316

321

324

@ferittuncer @jnyrup What do you guys think about merging it into TemperatureGradient (or TemperaturePerLength), and marking LapseRate as obsolete?

There is currently a v5 branch where we allow breaking changes, where we can remove anything made obsolete in v4.

982

jnyrup commented 3 years ago

I'm fine with obsoleting LapseRate as it is a quite meteorology specific unit and a special case of a temperature gradient.

It corresponds to the vertical component of the spatial gradient of temperature https://en.wikipedia.org/wiki/Lapse_rate

Some observations regard naming. UnitsNet has several PerLength units and only a single Gradient unit (ElectricCurrentGradient). Googling a bit it seems TemperatureGradient is an established term. UnitsNet has ForcePerLength for Surface Tension.

angularsen commented 2 years ago

@dglozano It seems we agree then, please go ahead with the PR:

dglozano commented 2 years ago

Have created a PR to implement the suggested changes 😄

Not sure how to copy the / operation for TemperatureDelta/Length without making a breaking change by removing the old one that gives a LapseRate as result.

Any suggestion on how to procede?

image

angularsen commented 2 years ago

Oh, that's right. Hm. I propose we allow that breaking change, since those updating will get a compile error (safe) and have an obsolete warning instructing them what to do.

0xferit commented 2 years ago

Hi, without knowing this domain I would favor moving everything to TemperatureGradient. It might be more consistent with our other quantities to name it TemperaturePerLength, but it seems TemperatureGradient is a well established quantity and maybe the better choice .

LapseRate has been touched in: #316 #321 #324

@ferittuncer @jnyrup What do you guys think about merging it into TemperatureGradient (or TemperaturePerLength), and marking LapseRate as obsolete?

There is currently a v5 branch where we allow breaking changes, where we can remove anything made obsolete in v4. #982

Hello @angularsen, I don't see a problem with this but my opinion might not be so helpful since I'm out of sync for since last 3 years.

angularsen commented 2 years ago

No problem @ferittuncer , I just thought I recognized your name in one of the PRs. It helps to know that you don't mind the change, anyhow 👍

dglozano commented 2 years ago

Solved in #991 , so I am closing this 😁