From d70dfb91ac890c1059e5043273f0e2c3e32ed209 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Fri, 24 Jul 2026 19:41:14 -0700 Subject: [PATCH 1/3] fix: give alternate unit suffixes their own keys Units whose UNECE symbol column lists aliases (e.g. "% or pct") were emitted twice under the SAME Python name, so the second assignment overwrote the first. units.PERCENT therefore resolved to the "pct" descriptor and the "%" one was unreachable as a module attribute. The same collision affected KILOGRAM_PER_LITRE, DECITONNE and RACK_UNIT. units_from_xls.py now keeps the canonical key for the first suffix and derives a distinct key for each alternate suffix; units.py is updated to match. Adds test/util/units_test.py covering the primary suffix, the distinct alias keys, and lookup by every suffix. Fixes #1250 --- bin/units_from_xls.py | 14 +++++++----- openhtf/util/units.py | 16 +++++++------- test/util/units_test.py | 47 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 test/util/units_test.py diff --git a/bin/units_from_xls.py b/bin/units_from_xls.py index 824f180c0..f061e00af 100644 --- a/bin/units_from_xls.py +++ b/bin/units_from_xls.py @@ -235,11 +235,15 @@ def unit_defs_from_sheet(sheet, column_names): continue seen.add(key) - # Split on ' or ' to support the units like '% or pct' - for suffix in suffix.split(' or '): - yield "%s = UnitDescriptor('%s', '%s', '''%s''')\n" % (key, name, code, - suffix) - yield 'ALL_UNITS.append(%s)\n' % key + # Split on ' or ' to support the units like '% or pct'. The first suffix + # keeps the canonical key; alternate suffixes get their own key so they + # do not overwrite it. + for idx, suffix in enumerate(suffix.split(' or ')): + alias_key = key if idx == 0 else unit_key_from_name( + '%s %s' % (name, suffix)) + yield "%s = UnitDescriptor('%s', '%s', '''%s''')\n" % (alias_key, name, + code, suffix) + yield 'ALL_UNITS.append(%s)\n' % alias_key except xlrd.XLRDError: sys.stdout.write('Unable to process the .xls file.') diff --git a/openhtf/util/units.py b/openhtf/util/units.py index 0c1cd4bbc..8bedcd174 100644 --- a/openhtf/util/units.py +++ b/openhtf/util/units.py @@ -659,8 +659,8 @@ class UnitDescriptor( ALL_UNITS.append(KILOGRAM_PER_CUBIC_DECIMETRE) KILOGRAM_PER_LITRE = UnitDescriptor('kilogram per litre', 'B35', '''kg/l''') ALL_UNITS.append(KILOGRAM_PER_LITRE) -KILOGRAM_PER_LITRE = UnitDescriptor('kilogram per litre', 'B35', '''kg/L''') -ALL_UNITS.append(KILOGRAM_PER_LITRE) +KILOGRAM_PER_LITRE_KG_PER_L = UnitDescriptor('kilogram per litre', 'B35', '''kg/L''') +ALL_UNITS.append(KILOGRAM_PER_LITRE_KG_PER_L) CALORIE_THERMOCHEMICAL_PER_GRAM = UnitDescriptor('calorie (thermochemical) per gram', 'B36', '''calth/g''') ALL_UNITS.append(CALORIE_THERMOCHEMICAL_PER_GRAM) KILOGRAM_FORCE = UnitDescriptor('kilogram-force', 'B37', '''kgf''') @@ -1379,8 +1379,8 @@ class UnitDescriptor( ALL_UNITS.append(DRY_TON) DECITONNE = UnitDescriptor('decitonne', 'DTN', '''dt''') ALL_UNITS.append(DECITONNE) -DECITONNE = UnitDescriptor('decitonne', 'DTN', '''dtn''') -ALL_UNITS.append(DECITONNE) +DECITONNE_DTN = UnitDescriptor('decitonne', 'DTN', '''dtn''') +ALL_UNITS.append(DECITONNE_DTN) DYNE = UnitDescriptor('dyne', 'DU', '''dyn''') ALL_UNITS.append(DYNE) PENNYWEIGHT = UnitDescriptor('pennyweight', 'DWT', '''''') @@ -2255,8 +2255,8 @@ class UnitDescriptor( ALL_UNITS.append(FRENCH_GAUGE) RACK_UNIT = UnitDescriptor('rack unit', 'H80', '''U''') ALL_UNITS.append(RACK_UNIT) -RACK_UNIT = UnitDescriptor('rack unit', 'H80', '''RU''') -ALL_UNITS.append(RACK_UNIT) +RACK_UNIT_RU = UnitDescriptor('rack unit', 'H80', '''RU''') +ALL_UNITS.append(RACK_UNIT_RU) MILLIMETRE_PER_MINUTE = UnitDescriptor('millimetre per minute', 'H81', '''mm/min''') ALL_UNITS.append(MILLIMETRE_PER_MINUTE) BIG_POINT = UnitDescriptor('big point', 'H82', '''bp''') @@ -3647,8 +3647,8 @@ class UnitDescriptor( ALL_UNITS.append(PAGE_ELECTRONIC) PERCENT = UnitDescriptor('percent', 'P1', '''%''') ALL_UNITS.append(PERCENT) -PERCENT = UnitDescriptor('percent', 'P1', '''pct''') -ALL_UNITS.append(PERCENT) +PERCENT_PCT = UnitDescriptor('percent', 'P1', '''pct''') +ALL_UNITS.append(PERCENT_PCT) COULOMB_PER_METRE = UnitDescriptor('coulomb per metre', 'P10', '''C/m''') ALL_UNITS.append(COULOMB_PER_METRE) KILOWEBER = UnitDescriptor('kiloweber', 'P11', '''kWb''') diff --git a/test/util/units_test.py b/test/util/units_test.py new file mode 100644 index 000000000..993639d7b --- /dev/null +++ b/test/util/units_test.py @@ -0,0 +1,47 @@ +# Copyright 2026 Google Inc. All Rights Reserved. + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at + +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Unit tests for the units module.""" + +import unittest + +from openhtf.util import units + + +class UnitsTest(unittest.TestCase): + + def test_percent_keeps_primary_suffix(self): + """units.PERCENT must expose '%', not the 'pct' alias.""" + self.assertEqual(units.PERCENT.suffix, '%') + self.assertEqual(units.PERCENT_PCT.suffix, 'pct') + + def test_suffix_aliases_have_distinct_keys(self): + """Alias suffixes must not overwrite the canonical unit attribute.""" + for primary, alias in ( + ('PERCENT', 'PERCENT_PCT'), + ('KILOGRAM_PER_LITRE', 'KILOGRAM_PER_LITRE_KG_PER_L'), + ('DECITONNE', 'DECITONNE_DTN'), + ('RACK_UNIT', 'RACK_UNIT_RU'), + ): + primary_unit = getattr(units, primary) + alias_unit = getattr(units, alias) + self.assertEqual(primary_unit.code, alias_unit.code) + self.assertNotEqual(primary_unit.suffix, alias_unit.suffix) + + def test_lookup_by_every_suffix(self): + self.assertIs(units.Unit('%'), units.PERCENT) + self.assertIs(units.Unit('pct'), units.PERCENT_PCT) + + +if __name__ == '__main__': + unittest.main() From 15a670074e6f976aa421f287a9fb464d05b8d81e Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:14:52 -0700 Subject: [PATCH 2/3] chore: re-trigger CLA check From b81fdd53b04bf8a745a6fc03eaaf3f651004c510 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Tue, 4 Aug 2026 23:12:54 -0700 Subject: [PATCH 3/3] fix: name only the primary unit symbol, look up the rest Only four UNECE rows list more than one symbol. Deriving a module-level name for each alternate produced awkward keys like KILOGRAM_PER_LITRE_KG_PER_L. Keep the canonical key for the primary symbol and add alternates to ALL_UNITS anonymously so they remain reachable via units.Unit(''). An explicit ALIAS_UNIT_KEYS table names the one alias worth exposing, PERCENT_PCT. --- bin/units_from_xls.py | 37 +++++++++++++++++++++++++++++-------- openhtf/util/units.py | 9 +++------ test/util/units_test.py | 31 ++++++++++++++++++++----------- 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/bin/units_from_xls.py b/bin/units_from_xls.py index f061e00af..494c364f7 100644 --- a/bin/units_from_xls.py +++ b/bin/units_from_xls.py @@ -146,6 +146,17 @@ def __call__(self, name_or_suffix): ''' SHEET_NAME = 'Annex II & Annex III' + +# Module-level names for alternate symbols, keyed by (common code, symbol). +# Only four rows in the spreadsheet list more than one symbol, so an explicit +# table is cheaper and more readable than deriving names from the symbol. An +# alternate symbol that is not listed here is still appended to ALL_UNITS and +# so remains reachable through units.Unit(''); it just gets no +# module-level name. +ALIAS_UNIT_KEYS = { + ('P1', 'pct'): 'PERCENT_PCT', +} + UNIT_KEY_REPLACEMENTS = { ' ': '_', ',': '_', @@ -235,15 +246,25 @@ def unit_defs_from_sheet(sheet, column_names): continue seen.add(key) - # Split on ' or ' to support the units like '% or pct'. The first suffix - # keeps the canonical key; alternate suffixes get their own key so they - # do not overwrite it. + # Split on ' or ' to support units like '% or pct'. The first symbol + # listed is the primary one and keeps the canonical key. Alternate + # symbols are appended to ALL_UNITS so they stay reachable via + # units.Unit(''), but they only get a module-level name when + # ALIAS_UNIT_KEYS says so -- deriving one from the suffix produces + # names like KILOGRAM_PER_LITRE_KG_PER_L that nobody would type. for idx, suffix in enumerate(suffix.split(' or ')): - alias_key = key if idx == 0 else unit_key_from_name( - '%s %s' % (name, suffix)) - yield "%s = UnitDescriptor('%s', '%s', '''%s''')\n" % (alias_key, name, - code, suffix) - yield 'ALL_UNITS.append(%s)\n' % alias_key + if idx == 0: + alias_key = key + else: + alias_key = ALIAS_UNIT_KEYS.get((code, suffix)) + if alias_key is None: + yield "ALL_UNITS.append(UnitDescriptor('%s', '%s', '''%s'''))\n" % ( + name, code, suffix) + else: + yield "%s = UnitDescriptor('%s', '%s', '''%s''')\n" % (alias_key, + name, code, + suffix) + yield 'ALL_UNITS.append(%s)\n' % alias_key except xlrd.XLRDError: sys.stdout.write('Unable to process the .xls file.') diff --git a/openhtf/util/units.py b/openhtf/util/units.py index 8bedcd174..42ee83d87 100644 --- a/openhtf/util/units.py +++ b/openhtf/util/units.py @@ -659,8 +659,7 @@ class UnitDescriptor( ALL_UNITS.append(KILOGRAM_PER_CUBIC_DECIMETRE) KILOGRAM_PER_LITRE = UnitDescriptor('kilogram per litre', 'B35', '''kg/l''') ALL_UNITS.append(KILOGRAM_PER_LITRE) -KILOGRAM_PER_LITRE_KG_PER_L = UnitDescriptor('kilogram per litre', 'B35', '''kg/L''') -ALL_UNITS.append(KILOGRAM_PER_LITRE_KG_PER_L) +ALL_UNITS.append(UnitDescriptor('kilogram per litre', 'B35', '''kg/L''')) CALORIE_THERMOCHEMICAL_PER_GRAM = UnitDescriptor('calorie (thermochemical) per gram', 'B36', '''calth/g''') ALL_UNITS.append(CALORIE_THERMOCHEMICAL_PER_GRAM) KILOGRAM_FORCE = UnitDescriptor('kilogram-force', 'B37', '''kgf''') @@ -1379,8 +1378,7 @@ class UnitDescriptor( ALL_UNITS.append(DRY_TON) DECITONNE = UnitDescriptor('decitonne', 'DTN', '''dt''') ALL_UNITS.append(DECITONNE) -DECITONNE_DTN = UnitDescriptor('decitonne', 'DTN', '''dtn''') -ALL_UNITS.append(DECITONNE_DTN) +ALL_UNITS.append(UnitDescriptor('decitonne', 'DTN', '''dtn''')) DYNE = UnitDescriptor('dyne', 'DU', '''dyn''') ALL_UNITS.append(DYNE) PENNYWEIGHT = UnitDescriptor('pennyweight', 'DWT', '''''') @@ -2255,8 +2253,7 @@ class UnitDescriptor( ALL_UNITS.append(FRENCH_GAUGE) RACK_UNIT = UnitDescriptor('rack unit', 'H80', '''U''') ALL_UNITS.append(RACK_UNIT) -RACK_UNIT_RU = UnitDescriptor('rack unit', 'H80', '''RU''') -ALL_UNITS.append(RACK_UNIT_RU) +ALL_UNITS.append(UnitDescriptor('rack unit', 'H80', '''RU''')) MILLIMETRE_PER_MINUTE = UnitDescriptor('millimetre per minute', 'H81', '''mm/min''') ALL_UNITS.append(MILLIMETRE_PER_MINUTE) BIG_POINT = UnitDescriptor('big point', 'H82', '''bp''') diff --git a/test/util/units_test.py b/test/util/units_test.py index 993639d7b..cdaed2760 100644 --- a/test/util/units_test.py +++ b/test/util/units_test.py @@ -25,18 +25,27 @@ def test_percent_keeps_primary_suffix(self): self.assertEqual(units.PERCENT.suffix, '%') self.assertEqual(units.PERCENT_PCT.suffix, 'pct') - def test_suffix_aliases_have_distinct_keys(self): - """Alias suffixes must not overwrite the canonical unit attribute.""" - for primary, alias in ( - ('PERCENT', 'PERCENT_PCT'), - ('KILOGRAM_PER_LITRE', 'KILOGRAM_PER_LITRE_KG_PER_L'), - ('DECITONNE', 'DECITONNE_DTN'), - ('RACK_UNIT', 'RACK_UNIT_RU'), + def test_primary_symbol_wins_the_canonical_key(self): + """The first symbol listed in the sheet keeps the module-level name.""" + for key, suffix in ( + ('PERCENT', '%'), + ('KILOGRAM_PER_LITRE', 'kg/l'), + ('DECITONNE', 'dt'), + ('RACK_UNIT', 'U'), ): - primary_unit = getattr(units, primary) - alias_unit = getattr(units, alias) - self.assertEqual(primary_unit.code, alias_unit.code) - self.assertNotEqual(primary_unit.suffix, alias_unit.suffix) + self.assertEqual(getattr(units, key).suffix, suffix) + + def test_alternate_symbols_stay_reachable_by_lookup(self): + """Alternates have no module attribute but must still resolve.""" + for suffix, name in ( + ('pct', 'percent'), + ('kg/L', 'kilogram per litre'), + ('dtn', 'decitonne'), + ('RU', 'rack unit'), + ): + unit = units.Unit(suffix) + self.assertEqual(unit.suffix, suffix) + self.assertEqual(unit.name, name) def test_lookup_by_every_suffix(self): self.assertIs(units.Unit('%'), units.PERCENT)