Defining a unit a second time does not override the original definition

[saumiJ]

Hey guys,

First off, thank you for building pint! Its simplicity and power are a pleasure to work with. :)

I recently stumbled on a use-case where I do the following:

1. Define a new unit
2. Redefine it differently

The redefinition does not work.

See for instance:

from pint import UnitRegistry

ureg = UnitRegistry(on_redefinition='warn')

# define a new unit
ureg.define('eyespan = 2 cm')

# this works as expected
print(ureg.Quantity(1.0, 'eyespan').to('cm'))  # should be 2.0 cm
print(ureg.Quantity(1.0, 'cm').to('eyespan'))  # should be 0.5 eyespan

# redefine it
ureg.define('eyespan = 5 cm')

# this doesn't work as expected
print(ureg.Quantity(1.0, 'eyespan').to('cm'))  # should be 5.0 cm
print(ureg.Quantity(1.0, 'cm').to('eyespan'))  # should be 0.2 eyespan

A bit of debugging led to the _get_root_units method in the file registry.py. In this line, if a conversion is cached, its cached value is returned. Plus, I couldn't find a way to not use the cache.

I'm sorry if I've missed a workaround.

[hgrecco]

Indeed, we do not check if the cache is still valid as it would add an unnecessary toll only needed when redefining units (which is a rare practice anyway).

A few comments though

1. By default, you should get a warning when you redefine a unit (you can change this behaviour by using the on_redefinition parameter of the Registry)
2. We do not have a way to invalidate the cache, but we do have a way to build it (partially at least) using the internal function _build_cache
3. I would not agree to invalidate the cache automatically upon conversion requested but I would agree to rebuild the cache upon redefinition. However, we would need to rething the API to allow temporarily disabling this in case of multiple redefinitions (building the cache is expensive).

[cpascual]

And why not just leave the current behaviour and provide a public API for explicitly rebuilding the cache?

It could be a ureg.rebuild_cache() method or an optional parameter to ureg.define()
In any case, the existence of this mechanism should be documented in ureg.define

[hgrecco]

I would argue that if you redefine a unit, rebuilding the cache should be the default and not rebuilding should be optional.

[jondoesntgit]

I agree with @hgrecco

[saumiJ]

I also agree with @hgrecco.

Side note: After some refactoring of my code, I have overcome the need to redefine units in the same unit-registry. So this issue does not cause me troubles anymore (just so the priority is realistic).