fjuniorr / flowmapper

Mappings between elementary flows
MIT License
0 stars 1 forks source link

Create Unit class #49

Closed fjuniorr closed 6 months ago

fjuniorr commented 6 months ago

[cmutel] I guess a safer way to handle units is also needed - something like a Unit class which handles equality and can spit out a conversion factor on its own. For example, I don't think that 'kg' currently matches 'kilogram'. But please let me put together some data on common conversions first.

[fjuniorr] The problem that I see handling the conversion with a Unit class would be in the cases when we are dealing with conversions between different dimensions (such as mass to volume) and we need information about the specific flows involved. You can see this for water in get_conversion_factor. I know you mentioned that overall these are hard to find so this could be a non-issue.

[cmutel] Yes. Ideally this would be provided in the glossaries of the different systems... I think we will have maybe 50 of these, but our rules should be designed around the normal unit conversions within dimensions, not the exceptions. You can see the units I have collected through the years here. Brightway does unit string normalization because I hate units like "a", and this is another set of flows we will need to match.

fjuniorr commented 6 months ago

I've started to work on this in https://github.com/fjuniorr/flowmapper/pull/51 because it was a good opportunity to create a better way to store original and normalized versions of flow properties (so that any alternative discussed in https://github.com/fjuniorr/flowmapper/issues/46#issuecomment-1843761812 is possible without hacks).

something like a Unit class which handles equality and can spit out a conversion factor on its own.

This is working and the tests show the gist of it:

from flowmapper.unit import Unit

def test_init():
    unit = Unit.from_dict(
        {"unitName": {"@xml:lang": "en", "#text": "M2A"}}, "unitName.#text"
    )
    assert unit.value == "square meter-year"
    assert unit.raw_value == "M2A"
    assert unit.raw_object == {"unitName": {"@xml:lang": "en", "#text": "M2A"}}

def test_equals_mass():
    u1 = Unit("kg")
    u2 = Unit("kilogram")

    assert u1 == u2

def test_conversion_factor():
    u1 = Unit("mg")
    u2 = Unit("kg")
    actual = u1.conversion_factor(u2)
    expected = 1e-06
    assert  actual == expected

I think we will have maybe 50 of these, but our rules should be designed around the normal unit conversions within dimensions, not the exceptions.

@cmutel do you think I can remove conversions between dimensions entirely? I left some failing tests until we reach a decision

FAILED tests/test_get_conversion_factor.py::test_get_conversion_factor_water - assert nan == 0.001
FAILED tests/test_match_mapped_name_differences_with_unit_conversion.py::test_match_non_ionic_state - assert not {'values_changed': {"root[0]['conversion_factor']": {'new_value': 1.2142857142857142, 'old_value': 1.0}}}

You can see the units I have collected through the years here. Brightway does unit string normalization because I hate units like "a", and this is another set of flows we will need to match.

I've used bw2io.units.normalize_units and bw2io.units.DEFAULT_UNITS_CONVERSION instead of bringing the lists to flowmapper.

fjuniorr commented 6 months ago

I've merged #51 and moved the actionable itens to other issues: