Removing unit conversion from ToString methods. ToUnit must be called… #546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… first. ToString now has a single responsibility of converting to quantity into a string representation.
angularsen
approved these changes
Nov 3, 2018
| /// <param name="significantDigitsAfterRadix">The number of significant digits after the radix point.</param> | ||
| /// <returns>String representation.</returns> | ||
| /// <param name="cultureName">Name of culture (ex: "en-US") to use for localization and number formatting. Defaults to <see cref="GlobalConfiguration.DefaultCulture" /> if null.</param> | ||
| public string ToString(InformationUnit unit, [CanBeNull] string cultureName, int significantDigitsAfterRadix) | ||
| public string ToString(string cultureName, int significantDigitsAfterRadix) |
There was a problem hiding this comment.
For future reference, significantDigitsAfterRadix is a param I think should be baked into the format string instead - similar to double.ToString("F2"). We did put in a whole lot of magic though on number formatting, to use scientific notation (1.01e5) for values smaller/larger than a certain threshold, as well as this "digits after radix" concept. Just some bits of info to bring into the whole format string discussion later.
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: b27ab41 29cbadf 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: 355f82f 40a551e 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: 4f50c03 49026b2 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: 876f31f b2c0097 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: 5f625cb 876f31f Author: Tristan Milnthorp <tristan.milnthorp@ansys.com> Date: Wed Oct 17 09:14:05 2018 -0400 Merge branch 'v4' into Dimensionless commit 876f31f Merge: 98fce99 466f68a 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: 1c02a3d 423ec39 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: 0830706 1c02a3d 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: 7cfca07 283c6b6 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: f914132 632d6af 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
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… first. ToString now has a single responsibility of converting to quantity into a string representation #545