Skip to content

Remove Scalar quantity#1680

Draft
angularsen wants to merge 1 commit into
masterfrom
agl-codex/remove-scalar
Draft

Remove Scalar quantity#1680
angularsen wants to merge 1 commit into
masterfrom
agl-codex/remove-scalar

Conversation

@angularsen

@angularsen angularsen commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Motivation

Scalar was added in #889 for #849 as a way to wrap plain user-entered counts or values in an IQuantity shape. The historical use case was essentially "double/float/etc in an IQuantity format", not a dimensional quantity with real units.

Recent discussion in #1679 re-raised whether Scalar should exist at all. Looking at the current codebase, Scalar has 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) and Scalar / Scalar -> double are awkward. For callers that need plain numeric values, double/decimal/QuantityValue are clearer; for dimensionless physical relationships, existing quantities such as Ratio remain available when they are semantically appropriate.

Changes

  • Remove Common/UnitDefinitions/Scalar.json.
  • Remove generated Scalar, ScalarUnit, resource, tests, and number-extension artifacts.
  • Regenerate aggregate quantity registration and IQuantity test lists without Scalar.
  • Remove the stale Scalar entry from Common/UnitEnumValues.g.json.

Breaking change

This removes the public Scalar quantity and ScalarUnit.Amount, including factory/accessor APIs such as Scalar.FromAmount(...), .Amount, parsing/formatting APIs, and number extensions such as 2.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.Tests
  • dotnet build UnitsNet.slnx
  • dotnet test UnitsNet.Tests --no-build
  • dotnet test UnitsNet.NumberExtensions.Tests --no-build
  • dotnet test UnitsNet.NumberExtensions.CS14.Tests --no-build

Refs #849, #1200, #1679.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review

This is a clean, well-scoped removal PR. Summary of findings below.

Breaking changes

Correctly flagged in the PR description. This removes public API: Scalar, ScalarUnit.Amount, Scalar.FromAmount(...), .Amount property, parsing/formatting APIs, and the 2.Amount() / 2.Amount number extensions. This is a major-version-worthy breaking change and should land with a corresponding entry in the next major version's release/upgrade notes (per Docs/README.md#upgrade-guides) so downstream consumers relying on Scalar have a documented migration path (e.g., "use double/decimal/QuantityValue, or Ratio for dimensionless physical relationships," as stated in the PR description).

Alignment with project criteria

This removal is well justified by .agents/criteria-for-adding-quantities-and-units.md, which explicitly states a quantity should not be "dimensionless/unitless (consider using Ratio)." Scalar violates this outright, and grepping the generated code confirms it never appears in any cross-quantity relation (e.g. IDivisionOperators<Scalar, X, Y> for other X/Y), unlike quantities such as Length which participate in many. So the "no meaningful semantic usage" claim in the motivation checks out.

Completeness of the removal

I verified via rg -n "\bScalar\b" across the whole repo (working tree already reflects the merge) that there are no dangling references left in docs, README, CHANGELOG, or code — the removal is thorough:

  • Common/UnitDefinitions/Scalar.json deleted, and the corresponding Scalar entry removed from Common/UnitEnumValues.g.json.
  • All 5 generated artifacts removed (quantity, unit enum, resource file, number extensions x2 including the CS14 preview variant, and their generated tests).
  • Aggregate registration lists correctly pruned: Quantity.g.cs (DefaultProvider quantity list and RegisterUnitConversions), IQuantityTests.g.cs (both the unit-assertion list and the QuantityInfo_IsSameAsStaticInfoProperty list).
  • Hand-written UnitsNet.Tests/CustomCode/ScalarTests.cs (subclass of the generated ScalarTestsBase) is also removed, since its base class no longer exists.

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 changes

There are no changes to CodeGen/Generators/, so this doesn't touch the "how" of code generation — only the "what" (the Scalar.json definition input is gone, so the generator's normal aggregate-list logic naturally stops emitting Scalar entries). Nothing to flag on generator correctness.

Nit

Minor, 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.

@angularsen

Copy link
Copy Markdown
Owner Author

@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 double in the UnitsNet API for scalar results.

I'd rather move towards QuantityValue as part of #1544 if it helps improve precision beyond what double can do.

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

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96%. Comparing base (a7443cf) to head (6cd1891).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant