Units do not allow ° symbol

[fohrt]

Please verify that this bug has NOT been raised before.

• I checked and didn't find a similar issue

Describe the bug*

I want to add temperatures as parameters, but whenever I add a unit that features the ° symbol in the name, I get an error.

This would not be a problem if the actual unit symbol was shown instead of the unit name because the unit symbol does allow the ° symbol.

Steps to Reproduce

Enforce Parameter Units has to be set to true for the errors to occur:

1. Go to Settings page
2. Go to Physical Units
3. Add a Unit
4. Name = °C, Definition = +273.15K
5. Error occurs: Unit name must be a valid identifier

Alternatively, a similar error can be reproduced by the following steps:

1. Define a new parameter template with unit °C
2. Go to Parts and select a part
3. Add a new parameter
4. Template = the just created parameter template, Data = 5 °C
5. Error occurs: Could not convert 5 °C to °C

Expected behaviour

No error should occur when defining a unit with the ° symbol.

Not the unit name, but the unit symbol should be displayed.

Deployment Method

• Docker
• Bare metal

Version Information

Version Information:

InvenTree-Version: 0.13.3
Django Version: 3.2.23
Commit Hash: e81349e
Commit Date: None
Commit Branch: stable
Database: sqlite3
Debug-Mode: False
Deployed using Docker: False
Platform: Linux-5.15.0-94-generic-x86_64-with-glibc2.31
Installer: PKG
Target: ubuntu:20.04
Active plugins: [{'name': 'InvenTreeBarcode', 'slug': 'inventreebarcode', 'version': '2.0.0'}, {'name': 'InvenTreeCoreNotificationsPlugin', 'slug': 'inventreecorenotificationsplugin', 'version': '1.0.0'}, {'name': 'InvenTreeCurrencyExchange', 'slug': 'inventreecurrencyexchange', 'version': '1.0.0'}, {'name': 'InvenTreeLabel', 'slug': 'inventreelabel', 'version': '1.0.0'}, {'name': 'InvenTreeLabelSheet', 'slug': 'inventreelabelsheet', 'version': '1.0.0'}, {'name': 'DigiKeyPlugin', 'slug': 'digikeyplugin', 'version': '1.0.0'}, {'name': 'LCSCPlugin', 'slug': 'lcscplugin', 'version': '1.0.0'}, {'name': 'MouserPlugin', 'slug': 'mouserplugin', 'version': '1.0.0'}, {'name': 'TMEPlugin', 'slug': 'tmeplugin', 'version': '1.0.0'}]

Please verify if you can reproduce this bug on the demo site.

• I can reproduce this bug on the demo site.

Relevant log output

No response

[SchrodingersGat]

@fohrt I believe this is running up against the limitations of the Pint conversion library - which is what we use in the backend.

Custom units must be "valid python identifiers" - which means no special symbols.

For example if I try to force InvenTree to do this:

[SchrodingersGat]

@fohrt I have actually found a way to hack it in - but it requires a change to the code - #6498

[fohrt]

Thanks for the fast response and fix!

Unfortunately, this fix does not work for me. I used the following steps to update the code:

1. Halted InvenTree web server and background worker processes
   

2. Changed the code in /opt/inventree/InvenTree/InvenTree/conversion.py according to the pull request
   

3. Executed inventree run invoke update
   

4. Restarted the InvenTree web server and background worker process
   

The console output of invoke update stated multiple pint redefinitions, such as:
Redefining 'degC' (<class 'pint.delegates.txt_defparser.plain.UnitDefinition'>)

Sadly, it still doesn't work for me. Perhaps my work flow for updating the code is wrong?

I also don't quite understand why this would work, as °C is already defined by pint:
pint/default_en.txt

Btw, not wanting to be a nitpicker, but Kelvin does not use the ° symbol anymore. Don't ask me why, but pint uses °K as well, although it is wrong: The line reg.define('degreeK = °K = degK = kelvin = Kelvin') in your fix should actually be reg.define('degreeK = K = degK = kelvin = Kelvin')

[SchrodingersGat]

@fohrt on review I was too hasty with that! I have #6502 on the way to revert the change. It should support °C anyway without that "fix"

[SchrodingersGat]

Ok so here is my workflow, I think that °C is actually supported out of the box...

Create a new Part Parameter Template

Seems to just work for me (running off master branch). Perhaps I have misunderstood your stated issue?

Note: This is without defining any custom units or anything like that

[fohrt]

This works for me too, but adding this parameter (with specified unit) to a part does fail.

The same thing happens when defining a parameter template in Kelvin and adding this parameter to a part in °C (which should trigger the automatic conversion of pint as far as I understood).

[SchrodingersGat]

Ok, I can reproduce this... And now am very deep in the weeds.

For future reference:

• Allow string parsing of offset units hgrecco/pint#1349
• OffsetUnitCalculusError on String Parsing of degC, degF hgrecco/pint#386

[fohrt]

I think the problem with °C and similar units is, that pint does not store these representations as an alias but rather as a symbol. I made a pretty ugly fix to support temperature units below. I highlighted the code changes with comments (FIX TEMPERATURE UNIT 1/2). Use with caution though, as I have not tested it properly.

File: InvenTree/InvenTree/conversion.py

def convert_physical_value(value: str, unit: str = None, strip_units=True):

    original = str(value).strip()

    # Ensure that the value is a string
    value = str(value).strip() if value else ''
    unit = str(unit).strip() if unit else ''

    # Error on blank values
    if not value:
        raise ValidationError(_('No value provided'))

    if unit:
        backup_value = value + unit
    else:
        backup_value = None

    ureg = get_unit_registry()

    # FIX TEMPERATURE UNITS BEGIN 1
    try:
        temperature_args = is_temperature(value)
        if temperature_args:
            value = ureg.Quantity(*temperature_args)  
            value = value.to(unit)     
    
        else:
            value = ureg.Quantity(value)

            if unit:
                if is_dimensionless(value):
                    magnitude = value.to_base_units().magnitude
                    value = ureg.Quantity(magnitude, unit)
                else:
                    value = value.to(unit)
     # FIX TEMPERATURE UNITS END 1
    
    
    except Exception:
        if backup_value:
            try:
                value = ureg.Quantity(backup_value)
            except Exception:
                value = None
        else:
            value = None

    if value is None:
        if unit:
            raise ValidationError(_(f'Could not convert {original} to {unit}'))
        else:
            raise ValidationError(_("Invalid quantity supplied"))

    # Calculate the "magnitude" of the value, as a float
    # If the value is specified strangely (e.g. as a fraction or a dozen), this can cause issues
    # So, we ensure that it is converted to a floating point value
    # If we wish to return a "raw" value, some trickery is required
    try:
        if unit:
            magnitude = ureg.Quantity(value.to(ureg.Unit(unit))).magnitude
        else:
            magnitude = ureg.Quantity(value.to_base_units()).magnitude

        magnitude = float(ureg.Quantity(magnitude).to_base_units().magnitude)
    except Exception as exc:
        raise ValidationError(_(f'Invalid quantity supplied ({exc})'))

    if strip_units:
        return magnitude
    elif unit or value.units:
        return ureg.Quantity(magnitude, unit or value.units)
    return ureg.Quantity(magnitude)


# FIX TEMPERATURE UNITS BEGIN 2
def convert_to_number(value):
  try:
    return int(value)
  except ValueError:
    try:
      return float(value)
    except ValueError:
      return value


def is_temperature(value):
    try:
        ureg = get_unit_registry()
        
        for unit in ureg.get_compatible_units('[temperature]'):
        
            # do not change order of if clauses!
            if str(unit) in value:
                return (convert_to_number(value.replace(str(unit), "")), unit)    
            
            for alias in list(ureg._units[str(unit)].aliases):
                if alias in value:
                    return (convert_to_number(value.replace(alias, "")), unit)
            
            if ureg._units[str(unit)].symbol in value:
                return (convert_to_number(value.replace(ureg._units[str(unit)].symbol, "")), unit)
            
        return False
    except:
        return False
# FIX TEMPERATURE UNITS END 2

[fohrt]

Logarithmic units will fail as well. For these units to work, the unit registry needs to be initialized with the autoconvert_offset_to_baseunit flag set to True

Explanation 1

[SchrodingersGat]

@fohrt I had found the autoconvert_offset_to_baseunit - not sure if it would have any unforeseen consequences to other conversion?

Setting this parameter seems to make temperature units just work. So it is a simple fix for sure. Any idea about other effects it might have?

[fohrt]

This is what the documentation of pint says about it:

As an alternative to raising an error, pint can be configured to work more relaxed via setting the UnitRegistry parameter autoconvert_offset_to_baseunit to true. In this mode, pint behaves differently:

1. Multiplication of a quantity with a single offset unit with order +1 by a number or ndarray yields the quantity in the given unit.
2. Before all other multiplications, all divisions and in case of exponentiation involving quantities with offset-units, pint will convert the quantities with offset units automatically to the corresponding base unit before performing the operation.

For the given use case, I think both behaviors will be fine. The first behavior seems to be just what we want. The second behavior should not be much of a problem because the conversion to the unit supplied in the parameter template is called explicitly at the end. So only intermediate results will be represented in the base unit.

One thing to note is that this will also enable support for logarithmic units (beta feature of pint).

[SchrodingersGat]

Yes that seems pretty reasonable. Do you think you would be able to submit a PR to implement this? In addition to the registry init update (single line of code) we should add some unit tests to ensure that a variety of temperature conversions are handled correctly:

InvenTree/InvenTree/InvenTree/tests.py

Line 43 in 7adf2e0

class ConversionTest(TestCase):

[weizhangny]

I encountered a similar issue. When I input a unit with a Chinese character, the system reported an error. If I used an English letter or string, it worked. My system was migrated from the older version which doesn't have this issue before; the new version 0.13.5 reported an error for old data.

after I defined the physical unit in the global setting - physical unit (Chinese character to English unit "piece"), the issue was solved.

[SchrodingersGat]

@weizhangny locale issues would be separate to the temperature symbol issue here - I would suggest you make a new issue for that. It would be a good idea to look into whether the pint library offers multi lingual support, as a starting point.