Skip to content

Add the grams force unit#1655

Open
MH-ZShearer wants to merge 3 commits intoangularsen:masterfrom
MH-ZShearer:add-gramforce
Open

Add the grams force unit#1655
MH-ZShearer wants to merge 3 commits intoangularsen:masterfrom
MH-ZShearer:add-gramforce

Conversation

@MH-ZShearer
Copy link
Copy Markdown
Contributor

Add the grams force unit

@claude
Copy link
Copy Markdown

claude Bot commented May 1, 2026

Code Review

Summary

Adds GramForce as a new unit to the existing Force quantity. The unit is well-established (1 gf = 0.001 kgf = 9.80665 × 10⁻³ N), widely used in small-force contexts (mechanical testing, postal scales, fine spring design), and a natural companion to the existing KilogramForce and TonneForce units. The addition is appropriate.

Conversion formulas

The formulas are correct but can be simplified to match the project style (prefer scientific notation directly):

"FromUnitToBaseFunc": "({x} * 9.80665) / 1e3",
"FromBaseToUnitFunc": "({x} / 9.80665) * 1e3",

Consider writing these as:

"FromUnitToBaseFunc": "{x} * 9.80665e-3",
"FromBaseToUnitFunc": "{x} / 9.80665e-3",

This also matches the multiplication/division convention described in CLAUDE.md (use multiplication for FromUnitToBaseFunc and division for FromBaseToUnitFunc).

Missing documentation

Other force units (KilogramForce, TonneForce, Dyn) include XmlDocSummary and XmlDocRemarks fields. Please add them for GramForce, for example:

"XmlDocSummary": "The gram-force is a unit of force equal to the magnitude of force exerted by a gram of mass in standard gravity (9.80665 m/s²). It is equal to 9.80665 × 10⁻³ N.",
"XmlDocRemarks": "https://en.wikipedia.org/wiki/Kilogram-force"

Tests

No tests were added for the new unit. The project generates tests automatically from the JSON definitions, but a quick sanity check in the test project (e.g., asserting Force.FromGramsForce(1000).KilogramsForce ≈ 1) would be good.

Overall

Looks good with the above minor tweaks. The unit is clearly defined, the conversion is correct, and it slots naturally alongside the existing mass-force units.

@claude
Copy link
Copy Markdown

claude Bot commented May 1, 2026

Code Review

Overview: Adds GramForce as a new unit to the existing Force quantity. Generated code looks correct.

✅ What's good

  • Valid, commonly-used unit (gram-force / pond)
  • Conversion formula is mathematically correct: 1 gf = 9.80665 mN
  • Consistent with existing KilogramForce approach
  • Russian locale abbreviation (гс) included

🔧 Suggestions

1. Missing XML documentationKilogramForce has XmlDocSummary and XmlDocRemarks. Please add similar metadata:

"XmlDocSummary": "The gram-force is equal to the magnitude of the force exerted on one gram of mass in a standard gravitational field (9.80665 m/s²). One gram-force equals 9.80665 mN.",
"XmlDocRemarks": "https://en.wikipedia.org/wiki/Gram-force"

2. Conversion formula style — The codebase prefers scientific notation for constants. Simplify to avoid parentheses:

"FromUnitToBaseFunc": "{x} * 9.80665e-3",
"FromBaseToUnitFunc": "{x} / 9.80665e-3"

This is equivalent to ({x} * 9.80665) / 1e3 but cleaner.

3. Test coverage — Only one line was added to ForceTests.cs. Please verify which test was added — if it's just a unit string test ([InlineData(1, "gf")]), that's sufficient for the custom tests, and the generated test base covers conversion correctness.

Overall

This is a straightforward addition of a well-known unit. With the XML doc and formula style fixes, this is ready to merge.

@claude
Copy link
Copy Markdown

claude Bot commented May 2, 2026

Code Review

Overview

Adds GramForce / GramsForce to the Force quantity.

Correctness

  • Conversion: ({x} * 9.80665) / 1e3 - 1 gf = 0.001 kg * 9.80665 m/s^2 = 0.00980665 N. Correct.
  • Test value: GramsForceInOneNewton => 0.1019716e3 = 101.9716 gf/N. Check: 1/0.00980665 ~= 101.9716. Correct.
  • Abbreviation gf is the standard symbol for gram-force.
  • Naming GramForce / GramsForce is consistent with KilogramForce / KilogramsForce.
  • Russian localization гс (грамм-сила) is provided.

Criteria Check

Gram-force is a well-documented unit (NIST, Wikipedia) used in chemistry, materials science, and everyday scale readings. It completes the force-of-gravity family alongside KilogramForce and TonneForce. Meets the criteria for addition.

Generated Code

Looks correct and consistent with other additive units. The UnitEnumValues.g.json assignment (GramForce = 16) is at the end of the ForceUnit block, which is fine.

Summary

Good addition. No issues.

@claude
Copy link
Copy Markdown

claude Bot commented May 2, 2026

Code Review

Adding GramForce to Force — gram-force is a legitimate, well-known unit (used in scale calibration, spring constants, surface tension, etc.) and fits naturally next to KilogramForce.

Looks good

  • Conversion factor is correct: 1 gf = 9.80665 × 10⁻³ N → ({x} * 9.80665) / 1e3.
  • Inverse is correct: ({x} / 9.80665) * 1e3.
  • Test value GramsForceInOneNewton => 0.1019716e3 (≈ 101.97 gf/N) is correct.
  • Enum value 16 added after TonneForce = 15.
  • Generated code changes look correct (NanoFramework, NumberExtensions, tests).

Minor issues

  1. Missing <remarks> with reference URL. The sibling unit KilogramForce has:

    "XmlDocRemarks": "https://en.wikipedia.org/wiki/Kilogram-force"
    

    Please add a similar remark for GramForce (the same Wikipedia page covers gram-force, or https://en.wikipedia.org/wiki/Kilogram-force#Gram-force).

  2. JSON placement. The unit was inserted after DecimalNewton in the JSON file; the file otherwise keeps units in roughly alphabetical order. Placing GramForce between Dyn and KilogramForce would improve consistency, though this is cosmetic.

Otherwise looks ready to merge once the remark is added.

@claude
Copy link
Copy Markdown

claude Bot commented May 2, 2026

Code Review

Overview

Adds GramForce unit to the Force quantity. This is a legitimate, widely-used unit (gram-force = force exerted by 1 gram of mass under standard gravity). The addition fits well alongside the existing KilogramForce and TonneForce.

Correctness ✅

  • Conversion factor is correct: 1 gf = 9.80665 × 10⁻³ N = 0.00980665 N
  • Test value 0.1019716e3 = 101.9716 is correct (1 N ≈ 101.972 gf)
  • Enum value 16 added correctly at end of UnitEnumValues.g.json
  • Generated code looks correct across NanoFramework, NumberExtensions, and test files

Issues

Missing XML documentation — all other units in Force.json include XmlDocSummary and XmlDocRemarks (Wikipedia link). GramForce has neither. Compare to KilogramForce:

"XmlDocSummary": "The kilogram-force, or kilopond, is equal to...",
"XmlDocRemarks": "https://en.wikipedia.org/wiki/Kilogram-force"

Please add something like:

"XmlDocSummary": "The gram-force is a unit of force equal to the gravitational force exerted on one gram of mass in a 9.80665 m/s² gravitational field (standard gravity). One gram-force is equal to 9.80665 × 10⁻³ newtons.",
"XmlDocRemarks": "https://en.wikipedia.org/wiki/Gram-force"

Minor: conversion formula style({x} * 9.80665) / 1e3 mixes styles. For consistency with TonneForce which uses {x} * (9.80665 * 1000), consider {x} * (9.80665 / 1e3). Both are mathematically equivalent.

Summary

Solid addition overall. Please add XmlDocSummary and XmlDocRemarks before merging.

@claude
Copy link
Copy Markdown

claude Bot commented May 2, 2026

PR Review: Add the grams force unit

Overview: Adds GramForce (gf) to the Force quantity. The gram-force is a legitimate, widely-used unit in physics and engineering (particularly where small forces are measured against gravitational acceleration).


Positive

  • The unit is well-defined: 1 gf = 9.80665 × 10⁻³ N (standard gravity applied to 1 gram)
  • Includes ru-RU localization, consistent with neighboring units like KilogramForce
  • Generated code is correct and complete (NanoFramework, NumberExtensions, tests)
  • Test value 0.1019716e3 = 101.9716 gf/N is correct

Conversion function style

"FromUnitToBaseFunc": "({x} * 9.80665) / 1e3",
"FromBaseToUnitFunc": "({x} / 9.80665) * 1e3",

The project convention is to prefer a single multiplication for FromUnitToBaseFunc and a single division for FromBaseToUnitFunc. The current form mixes operators unnecessarily. These simplify to:

"FromUnitToBaseFunc": "{x} * 9.80665e-3",
"FromBaseToUnitFunc": "{x} / 9.80665e-3",

This matches the style of neighboring units (KilogramForce uses {x} * 9.80665 and {x} / 9.80665) and makes the intent immediately readable.


Units criteria ✅

  • Well-documented (Wikipedia, SI derived)
  • Widely used in physics, chemistry, and precision engineering
  • Unambiguous standard conversion factor
  • Fits naturally alongside KilogramForce, TonneForce, Dyn

Overall: Good addition. Please simplify the conversion functions to single * / / operations before merging.

@MH-ZShearer
Copy link
Copy Markdown
Contributor Author

  1. JSON placement. The unit was inserted after DecimalNewton in the JSON file; the file otherwise keeps units in roughly alphabetical order. Placing GramForce between Dyn and KilogramForce would improve consistency, though this is cosmetic.

I was unable to locate this issue. It appears to already be between Dyn and KilogramForce.

Otherwise, I believe I addressed all of the comments and issues raised.

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