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

3 participants