Wrong unit representation when multiplying a composed unit

[gcardozo123]

• Barril version:
  1.3.0
• Python version:
  3.6.6
• Operating System:
  Windows 10 64-bits

Description

When you multiply a ratio unit by another unit, the new unit representation is wrong.
For example: kg/m3 * m3 should result (ideally) in kg or something like (kg . m3) / m3, but Barril outputs kg/m3.m3, here:

from barril.units import Scalar
Scalar(1, 'kg/m3') * Scalar(1, 'm3')
#output: Scalar(1.0, 'kg/m3.m3', 'density * volume')

[nicoddemus]

This also fails and seems to be related:

from barril.units import Scalar

u1 = Scalar(1, 'kg/m3')
print(u1.GetValue(u1.unit))

u2 = Scalar(1, 'kg/m3') * Scalar(1, 'm3')
print(u2.GetValue(u2.unit))

Output:

λ python barril-units.py
1.0
Traceback (most recent call last):
  File "barril-units.py", line 7, in <module>
    print(u2.GetValue(u2.unit))
  File "w:\ws\Souring\envs\souring20-py36\lib\site-packages\barril\units\_scalar.py", line 114, in GetAbstractValue
    return self._quantity.ConvertScalarValue(self._value, unit)
  File "w:\ws\Souring\envs\souring20-py36\lib\site-packages\barril\units\_quantity.py", line 670, in ConvertScalarValue
    return self.Convert(value, to_unit)
  File "w:\ws\Souring\envs\souring20-py36\lib\site-packages\barril\units\_quantity.py", line 689, in Convert
    self._composing_categories, self._composing_units, to_unit, value
  File "w:\ws\Souring\envs\souring20-py36\lib\site-packages\barril\units\unit_database.py", line 1171, in Convert
    category_or_quantity_type, from_unit, to_unit, value
  File "w:\ws\Souring\envs\souring20-py36\lib\site-packages\barril\units\unit_database.py", line 1059, in _ConvertWithExp
    "Can only convert one unit to another (not a composed unit at this point)"
barril.units.unit_database.ComposedUnitError: Can only convert one unit to another (not a composed unit at this point)

So a composed scalar can't convert to its own unit, it seems.

@fabioz any hints here?

[fabioz]

This is a known limitation at this moment (when dealing with units which are composed we don't automatically simplify the quantity type and have to work with a different API).

i.e.: the following works.

from barril.units import Scalar

u1 = Scalar(1, 'kg/m3')
print(u1.GetValue(u1.unit))

u2 = Scalar(1, 'kg/m3') * Scalar(1, 'm3')
print(u2.GetValue(u2.GetQuantity().GetComposingUnits()))

Regarding the way the unit should be shown to add the parenthesis to be more explicit, that should be reasonable to do (note that I don't consider any of those bugs, rather feature requests for a different behavior as it's working as intended).

[nicoddemus]

EDIT moved the discussion about simplification over to #16, this issue is about the bug in the original post which should be fixable according to @fabioz. 👍

I think we are talking about 3 different things (and sorry because my comment might have added to the list):

1. Automatic simplification of composed quantity types: Scalar(1, 'kg') * Scalar(1, 'm3') should collapse to Scalar(1, 'kg/m3'), simple/not composed because we have density already on the unit system.

2. How composed units are displayed: 'kg/m3' * 'm3' is displayed as "kg/m3.m3" today.

3. How a composed unit can't obtain the its value from its own unit: a composed scalar u2 blows up with u2.GetValue(u2.unit).

From my POV, 1 is a feature request, while 2 and 3 are clearly bugs.

1. To me this is a feature request.
2. kg/m3.m3 is kg/m6 to the user, which is clearly wrong, so to me this is a bug.
3. u2.GetValue(u2.unit) seems like a complete breach in API, so this also seems like a bug to me.

If the bugs are complex to solve or not is another matter, but from a user's point of view they are bugs IMO. Have no idea about how complex is to fix 1, but seems minor if 2) and 3) work as then 1) becomes almost an implementation detail.

What do you guys think?

[gcardozo123]

I agree with @nicoddemus, to me 2 and 3 look like bugs, specially considering that u2.GetValue(u2.unit) is using its own unit, even if that unit isn't known anywhere else I suppose it shouldn't raise an error at this point, but it should raise when the invalid attribution is made (earlier).