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

Add TemperatureGradient and deprecate LapseRate #991

Merged
merged 6 commits into from
Nov 23, 2021

Conversation

dglozano
Copy link
Contributor

I have added a new Quantity called TemperatureGradient and deprecated LapseRate, as agreed in #988

In addition, I did a small change in the CodeGen project to place the [Obsolete] attribute below the XML documentation, to avoid getting some warnings.

This is how it was generated before my change
image

I have a small issue though: after cloning the repo, and before doing any change at all, I run the CodeGen and I see tons of changes being made. It seems that much of the code was alphabetically ordered before, and the CodeGen by default in my machine overrides that (check a923584)

Any suggestion on how to solve this ?

@dglozano dglozano mentioned this pull request Nov 21, 2021
@angularsen
Copy link
Owner

Any suggestion on how to solve this ?

Huh, that is super weird. Seems you are on MacOS or Linux maybe, I am on Windows. Could it be anything there?

It should be alphabetically sorted. Could you debug the codegen console app and maybe see why it produces a different ordering? Maybe there is some subtle difference on LINQ not preserving the ordering or something. I would expect adding a OrderBy() at the right place should fix this.

@angularsen
Copy link
Owner

In addition, I did a small change in the CodeGen project to place the [Obsolete] attribute below the XML documentation, to avoid getting some warnings.

Sounds good to me 👍

@dglozano
Copy link
Contributor Author

That's right, I am using Intellij on a Mac. I will debug the CodeGen later today and try to sort it out 😀

Directory.GetFiles doesn't guarantee the order of the file names.
In windows it seems to be retrieving them alphabetically sorted,
but that's not the case for Mac.

Have taken the liberty to rename `jsonFile` to `jsonFileName`, since
the return of Directory.GetFiles is an array of file names. The content
of the file itself is read at a later point.
The [Obsolete] attribute was being generated on top of the XML documentation, which is an invalid location, so I have moved it right after it and before the
[DataContract] attribute.

Related:
 - angularsen#474
It will be replaced by TemperatureGradient
… from LapseRate

I have removed the TemperatureDelta/Length = LapseRate operator since it
would collide with the same overload that equals TemperatureGradient. This is
a breaking change, but it was decided that we can do it anyways since
it's easy to replace and you will get a compiler error.
@dglozano
Copy link
Contributor Author

It seems the culprit of the sorting issue was the Directory.GetFiles method that's used to read the quantity json files.
It doesn't guarantee the order of the files. It seems that the result in Windows is always sorted alphabetically, but that wasn't the case in Mac.

Ref.: https://docs.microsoft.com/en-us/dotnet/api/system.io.directory.getfiles?view=net-6.0
image

Have added the fix in this commit together with a small renaming.

@angularsen
Copy link
Owner

Great that you figured it out!

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me, just a few minor things I'll fix after merging.


protected override double DegreesCelciusPerMeterInOneKelvinPerMeter => 1;

protected override double DegreesFahrenheitPerFootInOneKelvinPerMeter => 0.54864;
Copy link
Owner

Choose a reason for hiding this comment

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

UnitsNet/UnitsNet.csproj Outdated Show resolved Hide resolved
@angularsen angularsen changed the title Add TemperatureGradient (and deprecate LapseRate) Add TemperatureGradient and deprecate LapseRate Nov 23, 2021
@angularsen angularsen merged commit 428bf7f into angularsen:master Nov 23, 2021
@angularsen
Copy link
Owner

Nuget should be out shortly.

Release UnitsNet/4.106.0 · 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.

2 participants