Add quantity CloudCover by MarkCiliaVincenti · Pull Request #883 · angularsen/UnitsNet

MarkCiliaVincenti · 2020-12-29T19:32:23Z

codecov · 2020-12-29T19:45:28Z

Codecov Report

Merging #883 (9ecac36) into master (f58656c) will decrease coverage by 0.04%.
The diff coverage is 76.92%.

@@ Coverage Diff @@## master #883 +/- ##
==========================================
- Coverage 82.76% 82.72% -0.05% 
==========================================
Files 287 291 +4 Lines 43362 43674 +312 ==========================================
+ Hits 35888 36128 +240 - Misses 7474 7546 +72

Impacted Files	Coverage Δ
UnitsNet/CustomCode/Quantities/CloudCover.extra.cs	`0.00% <0.00%> (ø)`
UnitsNet/GeneratedCode/UnitAbbreviationsCache.g.cs	`100.00% <ø> (ø)`
UnitsNet/GeneratedCode/Quantity.g.cs	`51.98% <50.00%> (-0.02%)`	⬇️
UnitsNet/InternalHelpers/UnitsNetMath.cs	`71.42% <71.42%> (ø)`
UnitsNet/GeneratedCode/Quantities/CloudCover.g.cs	`77.24% <77.24%> (ø)`
...ns/GeneratedCode/NumberToCloudCoverExtensions.g.cs	`100.00% <100.00%> (ø)`
UnitsNet/GeneratedCode/UnitConverter.g.cs	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f58656c...9ecac36. Read the comment docs.

…rriding of tolerance, added IntegerOktas property.

MarkCiliaVincenti · 2020-12-29T21:16:27Z

Not sure what the problem is with regards to the AppVeyor build failing. The code compiles for me and all cloud cover unit tests pass.

angularsen · 2020-12-29T21:47:21Z

Not sure what the problem is with regards to the AppVeyor build failing. The code compiles for me and all cloud cover unit tests pass.

It fails on

C:\projects\unitsnet\UnitsNet.WindowsRuntimeComponent\GeneratedCode\Quantities\CloudCover.g.cs(526,53): error CS0103: The name 'UnitsNetMath' does not exist in the current context

angularsen · 2020-12-29T21:55:26Z

Ah, it's the windows runtime component code generator.
You added a new extension method that is currently not added to the WRC code base.

This is a target that is largely deprecated, but kept around for new units via JSON changes.

Could you please duplicate the extension method to UnitsNet.WindowsRuntimeComponent so that its code also compiles?

MarkCiliaVincenti · 2020-12-30T07:38:17Z

Ah, it's the windows runtime component code generator.
You added a new extension method that is currently not added to the WRC code base.
This is a target that is largely deprecated, but kept around for new units via JSON changes.
Could you please duplicate the extension method to UnitsNet.WindowsRuntimeComponent so that its code also compiles?

Done.

MarkCiliaVincenti · 2021-01-13T16:43:40Z

Bump.

angularsen · 2021-01-13T18:25:55Z

Waiting on #880 to decide on naming. Bumped that thread.

lipchev · 2021-01-13T19:29:55Z

- <TargetPlatformVersion Condition=" '$(TargetPlatformVersion)' == '' ">10.0.16299.0</TargetPlatformVersion>
- <TargetPlatformMinVersion>10.0.10586.0</TargetPlatformMinVersion>
+ <TargetPlatformVersion Condition=" '$(TargetPlatformVersion)' == '' ">10.0.18362.0</TargetPlatformVersion>
+ <TargetPlatformMinVersion>10.0.18362.0</TargetPlatformMinVersion>


Is there a reason to bump up the MinVersion?

I think Visual Studio makes this change automatically. But yes, best to revert this.

lipchev · 2021-01-13T19:41:53Z

+ /// <param name="min">The minimum value allowed</param>
+ /// <param name="max">The maximum value allowed</param>
+ /// <returns>The clamped value</returns>
+ public static T Clamp<T>(this T val, T min, T max) where T : IComparable<T>


This would also work for any IQuantity - we could maybe expose it using a more restrictive (IQuantity) version in UnitMath

angularsen · 2021-01-13T19:46:08Z

As discussed in #880, let's go with DecimalFraction.

lipchev · 2021-01-13T20:23:26Z

Sorry I haven't read up much on the Oktas but doesn't it make more sense to have Clamp as a method on the quantity itself, rather than the internally stored value? Currently it seems that we cannot ensure proper addition/subtraction operations give the lossy nature of the internal round. For example isn't the result from (A + B)/2 likely to be (significantly) different from A/2 +B/2?
Anyway, If this is the desired behavior then I would think it is worth a test or two, demonstrating the specific (discrete?) nature of the quantity (like with the PowerRatioTests).

Also, maybe we should think about flagging the json file somehow, to indicate that it is non-linear (like with the Logarithmic flag)

angularsen · 2021-01-18T19:48:44Z

Currently it seems that we cannot ensure proper addition/subtraction operations give the lossy nature of the internal round.

@lipchev Good point, but then I propose we rather disable arithmetic codegen?
I don't think it makes a whole lot sense to do quarter cloud cover + half cloud cover anyway.

"GenerateArithmetic": false,

See Temperature as an example.

angularsen · 2021-01-18T19:53:32Z

@lipchev As for clamping, a better solution might be to add support in codegen for clamping of a quantity - so you could define that CloudCover should be clamped to say [0,1] of base unit - or some selected unit. Then it would inject clamping where appropriately. However, since this is the first quantity that requires it, I'm fine with doing it per unit for now. I think it should do what we want, to not allow less than 0% and not more than 100% cloud cover.

angularsen · 2021-01-18T19:57:36Z

@MarkCiliaVincenti I believe this is the current status of the PR feedback:

Rename to DecimalFraction
Don't generate code for arithmetic with "GenerateArithmetic": false, in JSON
Move clamp to UnitMath and if possible make it reusable for any IQuantity
Revert version change in UnitsNet.WindowsRuntimeComponent.csproj

stale · 2021-03-19T23:13:32Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

MarkCiliaVincenti · 2022-11-20T09:21:15Z

@MarkCiliaVincenti I believe this is the current status of the PR feedback:
Rename to DecimalFraction
Don't generate code for arithmetic with "GenerateArithmetic": false, in JSON
Move clamp to UnitMath and if possible make it reusable for any IQuantity
Revert version change in UnitsNet.WindowsRuntimeComponent.csproj

Sorry, I saw this almost 2 years late :)

What did you mean by renaming to DecimalFraction? Instead of CloudCover?

angularsen · 2022-11-20T20:24:47Z

In JSON, rename SingularName/PluralName Fraction/Fractions to DecimalFraction/DecimalFractions to be consistent with Ratio.DecimalFraction.

MarkCiliaVincenti added 2 commits December 29, 2020 09:58

Added preliminary CloudCover.json, requesting comments.
667b30e

first draft
1ee0618

angularsen reviewed Dec 29, 2020
View reviewed changes

Comment threadCommon/UnitDefinitions/CloudCover.json

angularsen reviewed Dec 29, 2020
View reviewed changes

Comment threadCommon/UnitDefinitions/CloudCover.json

angularsen reviewed Dec 29, 2020
View reviewed changes

Comment threadUnitsNet.Tests/CustomCode/CloudCoverTests.cs Outdated

angularsen changed the title ~~Cloud cover~~Add quantity CloudCoverDec 29, 2020

MarkCiliaVincenti added 2 commits December 29, 2020 21:57

Added NumericExtensions, clamped cloud cover, removed unnecessary ove…
20f3be9
…rriding of tolerance, added IntegerOktas property.

regenerated code; had to rename NumericExtensions
c960e62

angularsen mentioned this pull request Dec 30, 2020
Creating generic version of structs (WIP) #714
Closed

Duplicated extension method to UnitsNet.WindowsRuntimeComponent
9ecac36

lipchev reviewed Jan 13, 2021
View reviewed changes

staleBot added the wontfix label Mar 19, 2021

staleBot closed this Jun 2, 2021

Conversation

MarkCiliaVincenti commented Dec 29, 2020• edited by angularsen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecovBot commented Dec 29, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarkCiliaVincenti commented Dec 29, 2020

Uh oh!

angularsen commented Dec 29, 2020

Uh oh!

angularsen commented Dec 29, 2020

Uh oh!

MarkCiliaVincenti commented Dec 30, 2020

Uh oh!

MarkCiliaVincenti commented Jan 13, 2021

Uh oh!

angularsen commented Jan 13, 2021

Uh oh!

lipchevJan 13, 2021

Choose a reason for hiding this comment

Uh oh!

angularsenJan 13, 2021

Choose a reason for hiding this comment

Uh oh!

lipchevJan 13, 2021

Choose a reason for hiding this comment

Uh oh!

angularsen commented Jan 13, 2021

Uh oh!

lipchev commented Jan 13, 2021

Uh oh!

angularsen commented Jan 18, 2021

Uh oh!

angularsen commented Jan 18, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angularsen commented Jan 18, 2021

Uh oh!

staleBot commented Mar 19, 2021

Uh oh!

MarkCiliaVincenti commented Nov 20, 2022

Uh oh!

angularsen commented Nov 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MarkCiliaVincenti commented Dec 29, 2020•
edited by angularsen
Loading

codecovBot commented Dec 29, 2020•
edited
Loading

angularsen commented Jan 18, 2021•
edited
Loading