Skip to content

Si unit system - #524

Merged
angularsen merged 8 commits into
angularsen:v4from
tmilnthorp:SIUnitSystem
Oct 16, 2018
Merged

Si unit system#524
angularsen merged 8 commits into
angularsen:v4from
tmilnthorp:SIUnitSystem

Conversation

@tmilnthorp

Copy link
Copy Markdown
Collaborator

No description provided.

@tmilnthorp
Adding BaseUnits class. Adding SI unit system.
@tmilnthorp
tmilnthorp changed the base branch from master to v4October 15, 2018 19:41
@angularsenangularsen mentioned this pull request Oct 15, 2018

@angularsenangularsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Some minor things.

Comment threadUnitsNet/BaseUnits.cs Outdated
{
if(length == LengthUnit.Undefined)
throw new ArgumentException("The given length unit is undefined.", nameof(length));
else if(mass == MassUnit.Undefined)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No need to else when throwing.

Comment threadUnitsNet/BaseUnits.cs Outdated
/// <summary>
/// Represents the base units for a quantity or
/// </summary>
public sealed class BaseUnits

@angularsenangularsenOct 15, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Add IEquatable and IEquatable<BaseUnits>?

Comment threadUnitsNet/BaseUnits.cs Outdated
/// <returns>True if equal.</returns>
public static bool operator ==(BaseUnits left, BaseUnits right)
{
return left?.Equals(right) == true;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This fails to compare two null values I think.
https://stackoverflow.com/a/73732/134761

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Left hand side null is handled by return left?. The right hand side check is handled in the Equals method:

publicoverrideboolEquals(objectobj){if(objisnull||!(objisBaseUnits))returnfalse;

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I will add a test 😃

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

object left = null;
object right = null;
(left?.Equals(right) == true).Dump(); // False in linqpad

@tmilnthorptmilnthorpOct 16, 2018

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

That would not call BaseUnits.Equals, even if cast to null.

(BaseUnits)null==(BaseUnits)null// Does not call BaseUnits == operator

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I take that back. Forgot you can't step in.

I'll fix!

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Fixed :)

Comment threadUnitsNet/UnitSystem.cs Outdated
/// <summary>
/// Gets the SI unit system.
/// </summary>
public static UnitSystem SI { get; } = new UnitSystem(SIBaseUnits);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sweet.

@tmilnthorp
Implementing IEquatable<BaseUnits> on BaseUnits, and adding some tests for BaseUnits. Adding some doc too.
…th quantity for example has a BaseUnits where only Length would be set. Only a UnitSystem needs it all. Moving logic there. Expanding UnitSystem tests and logic. Fixing WRC.
@tmilnthorp
Fix for equality operator when both left and right are null
@tmilnthorp
Better add test for != too, in case someone changes the != implementation
@tmilnthorptmilnthorp mentioned this pull request Oct 16, 2018

@angularsenangularsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good stuff, just some comments.

Comment threadUnitsNet.Tests/BaseUnitsTests.cs Outdated
var baseUnits2 = new BaseUnits(LengthUnit.Meter, MassUnit.Kilogram, DurationUnit.Second,
ElectricCurrentUnit.Ampere, TemperatureUnit.Kelvin, AmountOfSubstanceUnit.Mole, LuminousIntensityUnit.Candela);

var baseUnits3 = new BaseUnits(LengthUnit.Foot, MassUnit.Pound, DurationUnit.Second,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No biggie, but I had to scan a bit to see what was different in these three. Could rename the variables to help read that. And you construct these three instance many times so we could have helpers like CreateDerivedUnitA() and CreateDerivedUnitB().

Comment threadUnitsNet/BaseDimensions.cs
Comment threadUnitsNet/BaseDimensions.cs Outdated
LuminousIntensity = luminousIntensity;
}

public bool IsBase()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

IsBaseUnit and IsDerivedUnit maybe? I had to think a bit when reading usages to get exactly what IsBase meant.

Comment threadUnitsNet/BaseDimensions.cs Outdated
LuminousIntensity = luminousIntensity;
}

public bool IsBase()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Add xmldoc to public members, that also helps with above comment.

Comment threadUnitsNet/BaseUnits.cs
Comment threadUnitsNet/BaseUnits.cs
Comment threadUnitsNet/BaseUnits.cs
Comment threadUnitsNet/UnitSystem.cs
@tmilnthorp
Adding doc. Making tests cleaner. Small method name changes,
Comment threadUnitsNet.Tests/BaseDimensionsTests.cs
@angularsen
angularsen merged commit 876f31f into angularsen:v4Oct 16, 2018
angularsen added a commit that referenced this pull request Nov 4, 2018
Squashed commit of the following:
commit 875ca71
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sun Nov 4 10:22:19 2018 +0100
UnitsNet: 4.0.0-alpha5
commit a05a86e
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Sat Nov 3 07:08:51 2018 -0400
Removing unit conversion from ToString methods. ToUnit must be called… (#546)
* Removing unit conversion from ToString methods. ToUnit must be called first. ToString now has a single responsibility of converting to quantity into a string representation.
* WRC fix
commit 9c43523
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Fri Nov 2 10:25:44 2018 -0400
Merge remote-tracking branch 'angularsen/master' into v4
commit 30363ce
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Fri Nov 2 14:57:35 2018 +0100
Delete Vector2/3
commit 799bfcc
Merge: b27ab4129cbadf
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Wed Oct 31 11:20:50 2018 -0400
Merge remote-tracking branch 'angularsen/master' into v4
commit 29cbadf
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Wed Oct 31 00:06:00 2018 -0400
Making Obsolete attribute message consistent across scripts (#544)
commit 28f0989
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Wed Oct 31 00:03:31 2018 -0400
Fix for explicit AbbreviationsWithPrefixes not lining up with the pre… (#542)
* Fix for explicit AbbreviationsWithPrefixes not lining up with the prefixes.
* Abort build when AbbreviationsWithPrefixes mismatches prefixes count. Will remove incorrect json after testing failure on AppVeyor.
* Build fails properly. Putting missing abbreviation back.
* Fix for count output
commit 157b288
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Tue Oct 30 12:27:39 2018 -0400
Fixing nasty bug in GetHashCode where two quantities with the same nu… (#541)
* Fixing nasty bug in GetHashCode where two quantities with the same numerical value/unit pair returned the same hash code. Type needed in GetHashCode implementation to guarantee uniqueness.
* Using cleanup
commit 0f4769d
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Mon Oct 29 17:08:16 2018 -0400
Adding dyn/cm² and lbm/(in·s²) units for pressure (#534)
* Adding dyn/cm² and lbm/(in·s²) units for pressure
* Adding microbars (= 1 dyn/cm²)
commit db10372
Author: Tristan Milnthorp <tmilnthorp@gmail.com>
Date: Mon Oct 29 16:54:59 2018 -0400
Adding support for micro/milli newtons, as well as ounce force (#535)
* Adding support for micro/milli newtons, as well as ounce force
* Fix for plural name and abbreviations for ounce force.
commit b27ab41
Merge: 355f82f40a551e
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Tue Oct 23 07:49:18 2018 +0200
Merge pull request #531 from tmilnthorp/IQuantityTAdditions
Adding additional functionality to IQuantity<UnitType>
commit 40a551e
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Mon Oct 22 17:00:57 2018 -0400
Adding additional functionality to IQuantity<UnitType>
commit 355f82f
Merge: 4f50c0349026b2
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Wed Oct 17 18:15:20 2018 +0200
Merge pull request #529 from tmilnthorp/CodeCleanup
Cleaning up some unused code and adding ValueTuple support to make un…
commit 4f50c03
Merge: 876f31fb2c0097
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Wed Oct 17 18:04:36 2018 +0200
Merge pull request #528 from tmilnthorp/Dimensionless
Dimensionless
commit 49026b2
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 11:55:11 2018 -0400
Adding ValueTuple package to WRC
commit a3d0435
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 11:43:40 2018 -0400
Cleaning up some unused code and adding ValueTuple support to make unit abbreviations a bit clearer
commit b2c0097
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 09:27:07 2018 -0400
Add test
commit 8ca0738
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 09:24:08 2018 -0400
Cleanup
commit a187b58
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 09:18:46 2018 -0400
Adding doc and tests
commit f1afdd8
Merge: 5f625cb876f31f
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Wed Oct 17 09:14:05 2018 -0400
Merge branch 'v4' into Dimensionless
commit 876f31f
Merge: 98fce99466f68a
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Tue Oct 16 22:48:16 2018 +0200
Merge pull request #524 from tmilnthorp/SIUnitSystem
Adds UnitSystem and SI base units.
commit 466f68a
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 16:31:32 2018 -0400
Adding doc. Making tests cleaner. Small method name changes,
commit 98fce99
Merge: 1c02a3d423ec39
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Tue Oct 16 21:33:56 2018 +0200
Merge pull request #526 from tmilnthorp/PruneGeneratedTests
Pruning generated tests
commit 5f625cb
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 15:13:58 2018 -0400
Implementing dimensionless base dimension
commit 423ec39
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 14:56:26 2018 -0400
Pruning generated tests
commit 53aafef
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 13:56:55 2018 -0400
Better add test for != too, in case someone changes the != implementation
commit 812dcfb
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 13:54:35 2018 -0400
Fix for equality operator when both left and right are null
commit 8fe2088
Merge: 08307061c02a3d
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 13:35:04 2018 -0400
Merge remote-tracking branch 'angularsen/v4' into SIUnitSystem
commit 0830706
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 13:33:08 2018 -0400
Adding base/derived check to BaseDimensions
commit 21d8459
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 12:55:50 2018 -0400
Fixing BaseUnits to not throw exception for undefined units. The Length quantity for example has a BaseUnits where only Length would be set. Only a UnitSystem needs it all. Moving logic there. Expanding UnitSystem tests and logic. Fixing WRC.
commit f3c90ed
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Tue Oct 16 10:49:18 2018 -0400
Implementing IEquatable<BaseUnits> on BaseUnits, and adding some tests for BaseUnits. Adding some doc too.
commit 1c02a3d
Merge: 7cfca07283c6b6
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Mon Oct 15 21:48:00 2018 +0200
Merge pull request #523 Replace GetHashCode implementation to match library
Replace GetHashCode implementation to match library. Fixing exception…
commit 7cfca07
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Mon Oct 15 21:42:06 2018 +0200
Remove internal parse methods (#521)
* Remove redundant internal parse methods
ParseInternal and TryParseInternal methods are redundant
since the same logic is already in QuantityParser.
* Remove internal ParseUnit methods
Same as for Parse methods. DRY it up.
* QuantityParser: Parse using given abbreviations cache
Replace delegates for external unit parsing code with UnitParser.
* Add null checking and make Trim() more explicit
Trimming inline deep in the code is not easy to spot.
Make it explicit, just after null checking.
Fixes test that assumed ArgNullException.
commit a110555
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Mon Oct 15 15:39:35 2018 -0400
Adding BaseUnits class. Adding SI unit system.
commit 283c6b6
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Mon Oct 15 15:08:15 2018 -0400
Prefer non-equal objects for this test :)
commit 17acea8
Author: Tristan Milnthorp <tristan.milnthorp@ansys.com>
Date: Mon Oct 15 15:03:37 2018 -0400
Replace GetHashCode implementation to match library. Fixing exceptions with null in methods/operators. Adding tests to validate.
commit c5c6e51
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Mon Oct 15 16:54:37 2018 +0200
Merge generator scripts for WRC and NetFramework (#520)
* Merge generator scripts for WRC and NetFramework
Handle WRC special cases in code generator scripts instead of in generated code.
This removes all the #if sections and merges the "Common" partial code back into
each of the WRC and NetFramework files so it is in one place and easy to read.
- Arrange huge quantity code into #regions, easier to navigate
- Move code generation into functions, easier to read
- Add Types.psm1 with types for intellisense on parsed JSON*
- Require PowerShell 5.1 (for classes)
* VS and VSCode struggle with intellisense when importing types with
'using module', this is relatively new and will probably improve later.
* Remove generated common code for quantities
* generate-code.bat: Set workdir to scripts folder
Needed for 'using module' in powershell to work, it can't have variables.
* Regen
* Fix WRC compile errors
Fix build warns about xmldoc
* Fix for code changes
commit 86b2efc
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Mon Oct 15 16:14:44 2018 +0200
Stricter parsing (#516)
* Add abbreviations ″ and ′ to inch and foot
Found in the wiki
* Rename to LengthTests.FeetInches.cs
* Length: Add TryParseFeetInches() and ParseFeetInches()
Since parsing is generally made more strict, this method adds back the special parsing
to handle strings like: 1' 2"
* QuantityParser: Make parsing more strict
- Only parse the format "{value} {unit abbreviation}" where the spacing is optional
- No longer support parsing multiple quantities like "1 ft 2 in", use Length.ParseFeetInches() for that
- Move Trim() to inner-most level
* Regen
* Trim input in TryParseFeetInches()
commit 485d028
Merge: f914132632d6af
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sun Oct 14 23:10:30 2018 +0200
Merge branch 'v4-fix-tests' into v4
commit 632d6af
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sat Oct 13 18:39:24 2018 +0200
UnitParser: Minor cleanup
commit be26999
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sat Oct 13 17:17:45 2018 +0200
Remove weird tests
commit 74e54cf
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sat Oct 13 23:35:14 2018 +0200
Use instance of UnitAbbrevationsCache in test
It broke other tests depending on sequence of running tests.
commit f914132
Author: Andreas Gullberg Larsen <andreas.larsen84@gmail.com>
Date: Sun Oct 14 22:38:17 2018 +0200
Ignore generated code except Length, Information, Level (#517)
Length is a well known quantity with 'double' value
Information is a well known quantity with 'decimal' value
Level is a well known quantity with logarithmic value
@angularsenangularsen mentioned this pull request Nov 8, 2018
Merged
25 tasks
angularsen added a commit that referenced this pull request Dec 16, 2018
# 4.0.0 Release
This PR will serve as the list of items to complete and it will be updated to show the progress before finally merged into `master` when completed. We all have busy schedules, so **if you want to help move this work forward then that is much appreciated!** ## The main theme is to reduce binary size
In two years it has grown from 280 kB to 1.4 MB and a lot of it is due to unnecessary syntactic sugar with many method overloads for various number types and nullable types - for every of our 800+ units! It simply adds up to a big total.
These items are chosen from #180, trying to include as many of the low hanging fruits as possible, while still keeping the list short and realistic to complete in a reasonably short time - we are all busy in our daily lives. We can always have more major version bumps later than trying to perfect it all now.
## Feature complete before November, merged before mid-December
I would like to aim for a "beta" pre-release nuget sometime in October with all the items completed, so we can have some time to test it before releasing the final, stable 4.0.0 version before Christmas holidays.
We should have the work items list more or less final before Monday, October 8th.
## Changes
#### Added
- [x] UnitSystem with a different meaning, now defines the base units for a unit system (#524)
#### Removed
- [x] Replace netstandard1.0 target with netstandard2.0, to avoid the extra dependencies (#477)
- [x] Remove code marked as `[Obsolete]`, such as `VolumeUnit.Teaspoon` (#490)
- [x] Remove nullable `From` factory methods (#483)
- [x] Remove extension methods on _nullable_ number types (#483)
- [x] Remove all number extension methods (#497)
- [x] Remove `Length2d`, replaced by `Area` (#501)
- [x] Remove static methods on `UnitSystem`, should use `UnitSystem.Default` instead (#496)
- [x] Remove unit parameter from `ToString()` methods (#546)
#### Changed
- [x] Throw exception on NaN values in static constructor methods, like `FromMeters()` (#502, see #176 (comment))
- [x] Throw if unit was not specified when constructing quantities, (#499 - #389 (comment))
- [x] Stricter parsing (#343 and #180 (comment))
- [x] Remove unit parameter from `ToString()` methods (#546)
- [x] Change Temperature arithmetic back to use base unit Kelvin (#550, see #518)
#### Renamed
- [x] Correct SingularName for some Flow unit definitions (#494, see #360)
- [x] Split/rename UnitSystem into UnitParser, GlobalConfiguration, UnitAbbreviationsCache (#511)
#### Fixed
- [x] Search for any TODO comments in the code to address, remove comment and either fix or create issue (5d24432)
- [x] Update README with v4 changes #498
- [x] Do not try/catch in UnitConverter.Try-methods (#506)
- [x] Do not try/catch in `Length.TryParse()` and for other quantities (#507)
- [x] Add/update changelog in wiki for v4 with some summary copied from this issue, some v3 to v4 migration instructions and briefly explain why some stuff were removed
## Milestones
- [x] Finalize work items list (Monday, October 8)
- [x] Release 4.0.0-beta1, feature complete (Wednesday, October 31)
- [ ] ~Show release notes and upgrade guide when upgrading to 4.x nuget, if possible~
- [x] Release 4.0.0 (Monday, December 17)
Sign up for freeto 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

@tmilnthorp@angularsen