Skip to content

Conversation

@m-messer
Copy link
Member

@m-messer m-messer commented Nov 17, 2025

UPDATED

This PR is a bug fix for the features added in #237.

Problem
$μA$, as microamps was not being processed properly, as the the unicode character μ was automatically being translated to micro as implemented in #237. So $μA$ and $microampere$ gets transformed to $A*micro$, instead of $microampere$.

The Bug
In unit_system_conversions.py on line 8 there is a set of SI prefixes, containing ('full name', 'short name', 'base 10 definition', (alternatives as a tuple)).

In utility/physical_quantity_utilitites.py line 237 is the code that creates all the combinations of units and prefixes, with all the alternatives, and returns four dictionaries:

  1. {'unit_short_name OR unit_alternative': 'unit_long_name'} - e.g. {'A': 'ampere', 'Ampere': 'ampere'}
  2. Base units and all units with the relevant prefixes - e.g. { 'decaha': 'decahectare', 'muha': 'microhectare', 'microha': 'microhectare'}
  3. The units with various ending (joule vs joules)
    4.The units with various endings with the relevant prefixes (see 2)

The issue was that the prefix + short name did not have a mapping to prefix + long name, which is used for the evaluation. For example, microA did not have a mapping for microampere.

The Fix
utility/physical_quantity_utilitites.py line 276 appends the mapping of prefix + shortname: prefix + longnameto theprefixed_units` dictionary. If the prefix + short name is not already a key in the dictionary that maps short unit names to long unit names.

Other changes

  • Refactored the preprocessing steps that were added in Feature/unicode support #237 from context/phyiscal_quantity.py to ultiltiy/physical_quantity_utilities.py to share the preprocessing with the preview implementation
  • evaluation_tests.py - Added a test for previewing unicode mu then evaluating there response
  • preview_implementations/physical_quantity_preview.py - Implemented the preprocessing for the units added in Feature/unicode support #237 to support unicode characters in preview
  • preview_tests.py - Removed old todo note
  • tests/physical_quantitiyes_evaluation_tests.py - edited test case to match the implementation. There should be no space between the prefix and the short name of the unit.
  • utility/physical_quantity_utilities.py - The fix discussed above on line 276, and the refactored code from context/physical_quantity.py make up the rest of the changes.

Copy link
Contributor

@peterbjohnson peterbjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent overview, thanks.

I've made a comment that you may want to address. Other than that, this change looks good.

# Conflicts:
#	app/evaluation_tests.py
@m-messer m-messer merged commit 73dd982 into main Dec 5, 2025
@m-messer m-messer deleted the fix/mu-physical-quanities branch December 5, 2025 13:24
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.

3 participants