pint icon indicating copy to clipboard operation
pint copied to clipboard

Addition `pint.Quantity + custom class` never defered to custom class's `__radd__`.

Open rwijtvliet opened this issue 2 years ago • 2 comments

Issue

TLDR

pint.Quantity.__add__ raises DimensionalityError which (IMHO) it shouldn't.

Detailed

In the example below, custom class ('GameDurations') implements __add__ and __radd__, so it can handle self + pint.Quantity and pint.Quantity + self. In the latter, __radd__ is not called: pint.Quantity.__add__ is (expectedly) called before GameDurations.__radd__. However, pint.Quantity.__add__ raises a DimensionalityError.

Example

from __future__ import annotations
from pint import Quantity as Q_


class GameDurations:
    def __init__(self, gamedurations: dict) -> None:
        self.durdict = gamedurations

    def __repr__(self) -> str:
        return "\n".join(f"{game}: {dur}" for game, dur in self.durdict.items())

    def __add__(self, other) -> GameDurations:
        return GameDurations({game: dur + other for game, dur in self.durdict.items()})

    __radd__ = __add__

games = GameDurations({"football": Q_("90 minutes"), "monopoly": Q_("7 hours")})
games
# football: 90 minute
# monopoly: 7 hour

# working as expected:
games + Q_("30 minutes")
# football: 120 minute
# monopoly: 7.5 hour

# not working, because Quantity raises Error, so GamesDuration.__radd__ is not called
Q_("30 minutes") + games
# DimensionalityError

Solution:

in quantity.py, change line 1048 to return NotImplemented.

rwijtvliet avatar May 16 '22 07:05 rwijtvliet

Pint-Pandas and xarray also had this issue, which was resolved by with upcast types that will return NotImplemented for operations like adding or multiplying.

see https://github.com/hgrecco/pint/blob/fe3e8f7f1ea3b27d6beeb52d44919c9cf6108bb0/pint/compat.py#L165

andrewgsavage avatar May 16 '22 11:05 andrewgsavage

@andrewgsavage Do you see a problem with the solution posted by @rwijtvliet ? Or a benefit in the upcast_types?

hgrecco avatar May 22 '22 15:05 hgrecco

I am closing this as the upcast type seems to be the preferred choice and I am not sure about the unintended consequences.

hgrecco avatar Apr 25 '23 15:04 hgrecco