Skip to content

Fix MolarFlow per-hour abbreviations - #1682

Merged
angularsen merged 1 commit into
masterfrom
agl-codex/fix-molar-flow-hour-abbreviations
Jul 17, 2026
Merged

Fix MolarFlow per-hour abbreviations#1682
angularsen merged 1 commit into
masterfrom
agl-codex/fix-molar-flow-hour-abbreviations

Conversation

@angularsen

Copy link
Copy Markdown
Owner

Motivation

Issue #1663 reports that the generated abbreviations for MolarFlow per-hour units are shifted by one kilo prefix:

  • MolarFlowUnit.MolePerHour formats/parses as kmol/h
  • MolarFlowUnit.KilomolePerHour formats/parses as kkmol/h

The root cause is that MolePerHour has Prefixes: [ "Kilo" ], but its base abbreviation in MolarFlow.json was already written as kmol/h. The prefix generator then prepended another k for the generated KilomolePerHour unit.

Changes

  • Change MolePerHour abbreviation from kmol/h to mol/h.
  • Regenerate MolarFlow resources and generated tests.
  • Generated abbreviations now match the other molar-flow units:
    • MolePerHour: mol/h
    • KilomolePerHour: kmol/h

Validation

  • generate-code.bat
  • dotnet test UnitsNet.Tests --filter "FullyQualifiedName~MolarFlowTests"

Fixes#1663.

@claude

claudeBot commented Jul 17, 2026

Copy link
Copy Markdown

Review

Clean, well-scoped bug fix for #1663. Verified the root cause and the fix directly.

Correctness

  • MolarFlow.json: MolePerHour had Abbreviations: ["kmol/h"] while also declaring Prefixes: ["Kilo"]. The prefix generator correctly prepends k to build KilomolePerHour's abbreviation from the base unit's abbreviation, so kmol/hkkmol/h. Changing the base abbreviation to mol/h is the right fix — it now matches the pattern used by every other unit in this file (mol/s/kmol/s, mol/min/kmol/min).
  • I checked the rest of Common/UnitDefinitions/*.json for the same class of bug (a prefixed unit whose base abbreviation already starts with its own prefix letter) and didn't find another instance — this looks like an isolated bug rather than a systemic generator issue, so no generator code changes were needed here, which matches what the diff shows.

Generated code

  • All three touched files (MolarFlow.json, MolarFlowTestsBase.g.cs, MolarFlow.restext) are consistent with a generate-code.bat run — the restext and test data mirror the JSON change exactly, nothing looks hand-edited. No generator logic was touched, consistent with this being a data-only fix.

Breaking change

  • Worth flagging explicitly (even though it's the correct fix): before this change, "kmol/h" parsed as MolarFlowUnit.MolePerHour; after this change, it parses as MolarFlowUnit.KilomolePerHour. Any consumer who was unknowingly relying on the old (buggy) parse/format behavior for MolePerHour will see a silent behavioral change, not a compile error. Given kkmol/h was obviously wrong, this is the right tradeoff, but it may be worth a one-line callout in the release notes since it changes parsing/formatting output rather than just fixing a compile-time API.

Test coverage

  • Good — the existing generated MolarFlowTestsBase.g.cs theory cases already cover Parse, TryParse, ParseUnit, TryParseUnit, and ToString for both en-US and the Swedish-fallback case, and all are updated consistently with the new abbreviations. No new test scaffolding needed since this is purely a data correction.

Overall: small, correct, well-validated fix. No concerns blocking merge.

@angularsen
angularsen marked this pull request as ready for review July 17, 2026 16:53
@angularsen
angularsen enabled auto-merge (squash) July 17, 2026 16:54
angularsen added a commit that referenced this pull request Jul 17, 2026
## Motivation
Backport of #1682 for the v5 maintenance branch.
Issue #1663 reports that the generated abbreviations for MolarFlow
per-hour units are shifted by one kilo prefix:
- `MolarFlowUnit.MolePerHour` formats/parses as `kmol/h`
- `MolarFlowUnit.KilomolePerHour` formats/parses as `kkmol/h`
The root cause is that `MolePerHour` has `Prefixes: [ "Kilo" ]`, but its
base abbreviation in `MolarFlow.json` was already written as `kmol/h`.
The prefix generator then prepended another `k` for the generated
`KilomolePerHour` unit.
## Changes
- Change `MolePerHour` abbreviation from `kmol/h` to `mol/h`.
- Regenerate MolarFlow resources and generated tests.
- Generated abbreviations now match the other molar-flow units:
- `MolePerHour`: `mol/h`
- `KilomolePerHour`: `kmol/h`
## Validation
- `dotnet run --project CodeGen -p:NuGetAudit=false`
- `dotnet test UnitsNet.Tests -f net48 --filter
"FullyQualifiedName~MolarFlowTests" -p:NuGetAudit=false`
- `dotnet test UnitsNet.Tests -f net9.0 -p:NuGetAudit=false`
Note: plain restore/codegen on this maintenance branch is currently
blocked by an unrelated NuGet audit warning for `NuGet.Protocol` 6.12.1,
so validation used `NuGetAudit=false` to avoid changing dependencies in
this bugfix PR.
Refs #1663.
@angularsen
angularsen merged commit 010ab2c into masterJul 17, 2026
4 checks passed
@angularsen
angularsen deleted the agl-codex/fix-molar-flow-hour-abbreviations branch July 17, 2026 17:10
@codecov

codecovBot 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 (cd518a7).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #1682 +/- ##
======================================
Coverage 96% 96% ======================================
Files 450 450 Lines 29306 29306 ======================================
Hits 28271 28271 Misses 1035 1035 

☔ 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 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.

Incorrect unit abbreviations for molar flow units

1 participant

@angularsen