Remove Scalar quantity#1680
Conversation
ReviewThis is a clean, well-scoped removal PR. Summary of findings below. Breaking changesCorrectly flagged in the PR description. This removes public API: Alignment with project criteriaThis removal is well justified by Completeness of the removalI verified via
This is exactly the kind of change that's easy to leave half-done (e.g. forgetting an aggregate list entry), but everything here appears consistent — nothing suggests a generator or template change, just data + regenerated output. No generator changesThere are no changes to NitMinor, non-blocking: the PR body's validation section says "Refs #849, #1200, #1679" — worth double-checking #1200 is actually relevant to this change (vs. #849 and #1679 which are directly referenced in the motivation) so reviewers/future readers aren't sent on a wild goose chase. Overall: solid, well-motivated removal with good hygiene. My main ask is just to make sure the breaking change is captured in upgrade/migration docs before release. |
|
@Muximize @lipchev Would like a second opinion here. Scalar is largely unused in our code base, and as discussed in #1679 (review) I don't believe Scalar is an improvement over I'd rather move towards However, looking at the original motivation for adding Scalar #849, I can see some minor convenience in offering it and it may just cause more harm removing it than keeping it. Thoughts? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1680 +/- ##
======================================
Coverage 96% 96%
======================================
Files 450 447 -3
Lines 29306 29207 -99
======================================
- Hits 28271 28189 -82
+ Misses 1035 1018 -17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Motivation
Scalarwas added in #889 for #849 as a way to wrap plain user-entered counts or values in anIQuantityshape. The historical use case was essentially "double/float/etc in anIQuantityformat", not a dimensional quantity with real units.Recent discussion in #1679 re-raised whether
Scalarshould exist at all. Looking at the current codebase,Scalarhas not grown meaningful semantic usage: it only appears as its own generated quantity, generated number extensions, generated tests, and aggregate registration entries. It is not used by quantity relations or handwritten domain code.Removing it avoids keeping a unitless value wrapper in the quantity model where APIs such as
scalar.As(ScalarUnit.Amount)andScalar / Scalar -> doubleare awkward. For callers that need plain numeric values,double/decimal/QuantityValueare clearer; for dimensionless physical relationships, existing quantities such asRatioremain available when they are semantically appropriate.Changes
Common/UnitDefinitions/Scalar.json.Scalar,ScalarUnit, resource, tests, and number-extension artifacts.IQuantitytest lists withoutScalar.Scalarentry fromCommon/UnitEnumValues.g.json.Breaking change
This removes the public
Scalarquantity andScalarUnit.Amount, including factory/accessor APIs such asScalar.FromAmount(...),.Amount, parsing/formatting APIs, and number extensions such as2.Amount()/2.Amount.Marked obsolete in v5: #1681
Validation
rg -n "\bScalar\b" Common UnitsNet UnitsNet.Tests UnitsNet.NumberExtensions UnitsNet.NumberExtensions.Tests UnitsNet.NumberExtensions.CS14 UnitsNet.NumberExtensions.CS14.Testsdotnet build UnitsNet.slnxdotnet test UnitsNet.Tests --no-builddotnet test UnitsNet.NumberExtensions.Tests --no-builddotnet test UnitsNet.NumberExtensions.CS14.Tests --no-buildRefs #849, #1200, #1679.