Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pint.Quantity.units / pint.Unit fails #2046

Open
peter-goldstein opened this issue Aug 15, 2024 · 1 comment
Open

pint.Quantity.units / pint.Unit fails #2046

peter-goldstein opened this issue Aug 15, 2024 · 1 comment

Comments

@peter-goldstein
Copy link

Steps to reproduce:

a = pint.Quantity(1, "L").units
b = pint.Unit("mL")
c = a / b

Expected behavior:
c is a dimensionless unit representing 1000

Actual behavior:
AttributeError: 'PlainUnit' object has no attribute '_get_non_multiplicative_units'


The offending line is in pint/facets/plain/quantity.py:999:

no_offset_units_other = len(other._get_non_multiplicative_units())

other does not have this method, because other is a Unit, and not a NonMultiplicativeQuantity. It looks like the bug is coming from quantity.py:989:

if isinstance(other, self._REGISTRY.Unit):
    other = 1 * other

This is evaluating to False, so other is remaining a Unit rather than being converted to a Quantity. I suspect this isn't quite the right logic for this conditional. I believe the intention is "is this item a Unit and attached to the same registry I am," but what it is actually checking is, "was this item constructed as an instance of my registry's inner class called Unit."


(Unnecessary) detail:

  • pint.Quantity(1, "L") invokes SharedRegistryObject.__new__, which gives the created object a field _REGISTRY pointing to the application registry
  • The units() property returns a self._REGISTRY.Unit, which does not have a _REGISTRY field but is an instance of an inner class of the application registry
  • pint.Unit("mL") invokes SharedRegistryObject.new`, so again it has the application registry as a field, but it isn't an instance of that registry's inner Unit class
@andrewgsavage
Copy link
Collaborator

I'd expect to get c = Quantity(1000, "dimensionless")

Looks like you're most of the way there, I'd change that to
if isinstance(other, self._REGISTRY.Unit) or (isinstance(other, Unit) and self._REGISTRY==other._REGISTRY):
A PR with a fix is welcomed.
the only other place I see an isinstance check with a Unit is L918 in quantity.py so change that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants