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 Scalar quantity #889

Merged
merged 2 commits into from
Jan 6, 2021
Merged

Add Scalar quantity #889

merged 2 commits into from
Jan 6, 2021

Conversation

Joe-houghton
Copy link
Contributor

@Joe-houghton Joe-houghton commented Jan 5, 2021

Fixes #849

e.g. Scalar.FromAmount(1.5).

e.g. Scalar.FromAmount(1.5).
@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Merging #889 (6ef552c) into master (ad2cd94) will decrease coverage by 0.05%.
The diff coverage is 73.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
- Coverage   82.76%   82.70%   -0.06%     
==========================================
  Files         287      289       +2     
  Lines       43362    43646     +284     
==========================================
+ Hits        35888    36098     +210     
- Misses       7474     7548      +74     
Impacted Files Coverage Δ
UnitsNet/GeneratedCode/UnitAbbreviationsCache.g.cs 100.00% <ø> (ø)
UnitsNet/GeneratedCode/Quantity.g.cs 51.98% <50.00%> (-0.02%) ⬇️
UnitsNet/GeneratedCode/Quantities/Scalar.g.cs 74.27% <74.27%> (ø)
...nsions/GeneratedCode/NumberToScalarExtensions.g.cs 100.00% <100.00%> (ø)
UnitsNet/GeneratedCode/UnitConverter.g.cs 100.00% <100.00%> (ø)
UnitsNet/GeneratedCode/Quantities/Area.g.cs 84.46% <0.00%> (ø)
UnitsNet/GeneratedCode/Quantities/Mass.g.cs 87.38% <0.00%> (ø)
UnitsNet/GeneratedCode/Quantities/Angle.g.cs 82.01% <0.00%> (ø)
UnitsNet/GeneratedCode/Quantities/Force.g.cs 83.68% <0.00%> (ø)
UnitsNet/GeneratedCode/Quantities/Level.g.cs 76.67% <0.00%> (ø)
... and 101 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 ad2cd94...6ef552c. Read the comment docs.

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.

Great! Just a few minor comments.

Common/UnitDefinitions/Scalar.json Show resolved Hide resolved
/// Purposely did not call it an Int as it is not an int underneath.
/// </summary>
[Fact]
public void ScalarWholeNumberEqualsScalarWholeNumber()
Copy link
Owner

Choose a reason for hiding this comment

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

I love tests, but I believe these test cases are already covered by the generated tests:

  • ArithmeticOperators
  • ConversionRoundTrip

https://github.com/angularsen/UnitsNet/pull/889/files#diff-136cd525eae5af5b9f86b098cdab2bc0741d8119d188b00e60524aada8d407a5R184-R202

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not see those, the one I did see was
Assertion(3, ScalarUnit.Amount, Quantity.From(3, ScalarUnit.Amount)); from IQuantityTests.g.cs

Looking from someone not familiar with the codebase one thing that I will add about the tests that you listed (ArithmeticOperators & ConversionRoundTrip) - they only use integers to test. Looking at the code underneath, it will not make a difference as they are stored as a double anyway, but if that implementation ever changes there may not be sufficient test coverage. I think this is outside of the scope of this PR, so I will remove those tests.

@angularsen angularsen changed the title Scalar Type added Add Scalar quantity Jan 5, 2021
@angularsen angularsen merged commit 9730bb4 into angularsen:master Jan 6, 2021
@angularsen
Copy link
Owner

Thanks a lot! A nuget is on the way.

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

Unit for "Amount of times"
2 participants