Repository: stravalib/stravalib

Issue:
Unithelper units don't support arithmetic and comparison operations
Based on legacy unithelper behavior, and pint out-of-the-box, the following examples should work (and return `True`):

```python
import unithelper as uh

uh.meters(10) == uh.meters(10)
uh.meters(20) > uh.meters(10)
uh.meters(20) + uh.meters(10) == uh.meters(30)
```

See https://github.com/oliverkurth/runquery/issues/6


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/stravalib/stravalib/pull/336?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) Report
> Merging [#336](https://codecov.io/gh/stravalib/stravalib/pull/336?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) (29b0ba7) into [master](https://codecov.io/gh/stravalib/stravalib/commit/2f42b33df911474cc25b5392b24292ff9748d6e5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) (2f42b33) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #336      +/-   ##
==========================================
- Coverage   73.62%   73.60%   -0.02%     
==========================================
  Files           8        8              
  Lines        1706     1705       -1     
  Branches      227      228       +1     
==========================================
- Hits         1256     1255       -1     
+ Misses        400      399       -1     
- Partials       50       51       +1     
```


| [Impacted Files](https://codecov.io/gh/stravalib/stravalib/pull/336?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) | Coverage Δ | |
|---|---|---|
| [stravalib/unithelper.py](https://codecov.io/gh/stravalib/stravalib/pull/336?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib#diff-c3RyYXZhbGliL3VuaXRoZWxwZXIucHk=) | `98.24% <100.00%> (+4.80%)` | :arrow_up: |
| [stravalib/attributes.py](https://codecov.io/gh/stravalib/stravalib/pull/336?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib#diff-c3RyYXZhbGliL2F0dHJpYnV0ZXMucHk=) | `79.76% <0.00%> (-1.41%)` | :arrow_down: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib)

oliverkurth: Thank you for working on this!

I just this PR tried with my code, and now I am getting a different error:
```
...
  File "/usr/lib/python3/dist-packages/flask/app.py", line 1935, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/okurth/runquery/runquery/query.py", line 611, in api_set_search
    stats = calc_stats(athlete, matched)
  File "/home/okurth/runquery/runquery/query.py", line 416, in calc_stats
    stats_total['dist'] += a.distance
  File "/usr/lib/python3/dist-packages/pint/quantity.py", line 965, in __iadd__
    return self._add_sub(other, operator.add)
  File "/usr/lib/python3/dist-packages/pint/quantity.py", line 106, in wrapped
    result = f(self, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/pint/quantity.py", line 859, in _add_sub
    if not self._check(other):
  File "/usr/lib/python3/dist-packages/pint/util.py", line 782, in _check
    raise ValueError(
ValueError: Cannot operate with Quantity and Quantity of different registries.
```
Same code line as before in https://github.com/oliverkurth/runquery/issues/6
```
stats_total['dist'] += a.distance
```

jsamoocha: The following test reproduced your issue:

```python
def test_get_activities_quantity_addition(mock_strava_api, client):
    mock_strava_api.get(
        '/athlete/activities',
        response_update={'distance': 1000.0},
        n_results=2
    )
    act_list = list(client.get_activities(limit=2))
    total_d = uh.meters(0)
    total_d += act_list[0].distance
    total_d += act_list[1].distance
    assert total_d == uh.meters(2000.)
```

I think we now have a cleaner and less hacky solution, and I have learned more about pint :-)
oliverkurth: Thanks again!

That fixed it, and I get a lot further. Now I am getting this error:
```
  File "/usr/lib/python3/dist-packages/flask/app.py", line 1935, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/okurth/runquery/runquery/query.py", line 612, in api_set_search
    stats_localized = stats_localize(athlete, stats)
  File "/home/okurth/runquery/runquery/query.py", line 459, in stats_localize
    'pace': units.unit('min')/unithelper.miles
  File "/home/okurth/.local/lib/python3.8/site-packages/units/leaf_unit.py", line 64, in __truediv__
    return ComposedUnit([self], [other])
  File "/home/okurth/.local/lib/python3.8/site-packages/units/composed_unit.py", line 93, in __new__
    (squeezed_numer, squeezed_denom, squeezed_multiplier) = squeeze(numer,
  File "/home/okurth/.local/lib/python3.8/site-packages/units/composed_unit.py", line 78, in squeeze
    (simpler_numer, simpler_denom, cancellation_mult) = cancel(result_numer,
  File "/home/okurth/.local/lib/python3.8/site-packages/units/composed_unit.py", line 34, in cancel
    if compatible(nleaf, dleaf):
  File "/home/okurth/.local/lib/python3.8/site-packages/units/compatibility.py", line 7, in compatible
    return unit1.canonical() == unit2.canonical()
AttributeError: 'UnitConverter' object has no attribute 'canonical'
```
This is the code:
```def stats_localize(athlete, stats):

    if athlete.measurement_preference != 'feet':
        ath_units = {
            'dist': unithelper.kilometers,
            'time' : unithelper.seconds,
            'elevation': unithelper.meters,
            'speed': unithelper.kph,
            'pace': units.unit('min')/unithelper.kilometers,
        }
    else:
        ath_units = {
            'dist': unithelper.miles,
            'time' : unithelper.seconds,
            'elevation' : unithelper.feet,
            'speed': unithelper.mph,
            'pace': units.unit('min')/unithelper.miles
        }
```
This is where it fails: ` 'pace': units.unit('min')/unithelper.miles`. But I think I need to fix this in my code, it shouldn't use `units` any more.
oliverkurth: Alright, I got my code working now with this PR, after a few minor changes.

Just one more thing - this:
```
miles_per_hour = mph = UnitConverter('mi/h')
```
should be 
```
miles_per_hour = mph = UnitConverter('mi/hour')
```
Looks like `h` has another meaning in `pint`.

Also
```
hour = hours = UnitConverter('h')
```
should be
```
hour = hours = UnitConverter('hour')
```

oliverkurth: > Do you have a test or code example that I can use to reproduce 'h' being misinterpreted?

I can repro on the console. Error with my code was similar.
```
$ python3
Python 3.8.10 (default, Nov 14 2022, 12:59:47) 
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pint
>>> ureg = pint.UnitRegistry()
>>> v = 3 * ureg.meters / (1 * ureg.seconds)
>>> ureg.define("mph_h = miles/h")
>>> v.to("mph_h")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/pint/quantity.py", line 563, in to
    magnitude = self._convert_magnitude_not_inplace(other, *contexts, **ctx_kwargs)
  File "/usr/lib/python3/dist-packages/pint/quantity.py", line 512, in _convert_magnitude_not_inplace
    return self._REGISTRY.convert(self._magnitude, self._units, other)
  File "/usr/lib/python3/dist-packages/pint/registry.py", line 884, in convert
    return self._convert(value, src, dst, inplace)
  File "/usr/lib/python3/dist-packages/pint/registry.py", line 1665, in _convert
    return super()._convert(value, src, dst, inplace)
  File "/usr/lib/python3/dist-packages/pint/registry.py", line 1279, in _convert
    return super()._convert(value, src, dst, inplace)
  File "/usr/lib/python3/dist-packages/pint/registry.py", line 917, in _convert
    raise DimensionalityError(src, dst, src_dim, dst_dim)
pint.errors.DimensionalityError: Cannot convert from 'meter / second' ([length] / [time]) to 'mph_h' ([time] / [length] / [mass])
>>> ureg.define("mph_hour = miles/hour")
>>> v.to("mph_hour")
<Quantity(6.710808876163206, 'mph_hour')>
```


oliverkurth: See also here: https://github.com/hgrecco/pint/issues/719 and https://github.com/hgrecco/pint/pull/1454. They changed it back and forth, and I have an older version. I think it's safer to just use 'hour' instead of 'h', there is no ambiguity.
lwasser: i'm happy if we merge this given @oliverkurth has tested on their workflow. if you are concerned about something specific and want a second tester i can test however. 

Patch:
diff --git a/changelog.md b/changelog.md
index cd9a6ff..c49398c 100644
--- a/changelog.md
+++ b/changelog.md
@@ -7,6 +7,7 @@
 * Add: Pre-commit hook + instructions and configure precommit.ci bot (@lwasser, #293)
 * Fix: Setuptools_scm bug when installing stravalib remotely via GitHub (@lwasser, #331)
 * Fix: fix LatLon unmarshal from string type (@oliverkurth, #334)
+* Fix: allows arithmetic and comparison between multiple quantities (jsamoocha, #335)
 
 ## v1.1.0
 
diff --git a/stravalib/unit_registry.py b/stravalib/unit_registry.py
new file mode 100644
index 0000000..615d7d2
--- /dev/null
+++ b/stravalib/unit_registry.py
@@ -0,0 +1,4 @@
+from pint import UnitRegistry
+
+ureg = UnitRegistry()
+Q_ = ureg.Quantity
diff --git a/stravalib/unithelper.py b/stravalib/unithelper.py
index 6a722cf..aa7bc14 100644
--- a/stravalib/unithelper.py
+++ b/stravalib/unithelper.py
@@ -9,6 +9,7 @@ from typing import Any, Protocol, Union, runtime_checkable
 import pint
 
 from stravalib.exc import warn_units_deprecated
+from stravalib.unit_registry import Q_
 
 
 @runtime_checkable
@@ -22,58 +23,46 @@ class UnitsQuantity(Protocol):
     unit: str
 
 
-class Quantity:
+class Quantity(Q_):
     """
-    Wraps `pint.Quantity`. Subclassing didn't work because pint has some heavily
-    customized attribute lookup
+    Extension of `pint.Quantity` for temporary backward compatibility with
+    the legacy `units` package.
     """
 
-    def __init__(self, q: pint.Quantity):
-        self.q = q
-
     @property
     def num(self):
         warn_units_deprecated()
-        return self.q.magnitude
+        return self.magnitude
 
     @property
     def unit(self):
         warn_units_deprecated()
-        return str(self.q.units)
+        return str(self.units)
 
     def __int__(self):
-        return int(self.q.magnitude)
+        return int(self.magnitude)
 
     def __float__(self):
-        return float(self.q.magnitude)
-
-    def __getattr__(self, item):
-        return getattr(self.q, item)
-
-    def __str__(self):
-        return str(self.q)
-
-    def __repr__(self):
-        return repr(self.q)
+        return float(self.magnitude)
 
 
 class UnitConverter:
     def __init__(self, unit: str):
-        self.unit = pint.Unit(unit)
+        self.unit = unit
 
     def __call__(self, q: Union[Number, pint.Quantity, UnitsQuantity]):
         if isinstance(q, Number):
             # provided quantity is unitless, so mimick legacy `units` behavior:
-            converted_q = self.unit * q
+            converted_q = Quantity(q, self.unit)
         else:
             try:
                 converted_q = q.to(self.unit)
             except AttributeError:
                 # unexpected type of quantity, maybe it's a legacy `units` Quantity
                 warn_units_deprecated()
-                converted_q = (pint.Unit(str(q.unit)) * q.num).to(self.unit)
+                converted_q = Quantity(q.num, q.unit).to(self.unit)
 
-        return Quantity(converted_q)  # adds legacy `units` behavior
+        return converted_q
 
 
 def is_quantity_type(obj: Any):
@@ -88,14 +77,14 @@ def is_quantity_type(obj: Any):
 
 meter = meters = UnitConverter('m')
 second = seconds = UnitConverter('s')
-hour = hours = UnitConverter('h')
+hour = hours = UnitConverter('hour')
 foot = feet = UnitConverter('ft')
 mile = miles = UnitConverter('mi')
 kilometer = kilometers = UnitConverter('km')
 
 meters_per_second = UnitConverter('m/s')
-miles_per_hour = mph = UnitConverter('mi/h')
-kilometers_per_hour = kph = UnitConverter('km/h')
+miles_per_hour = mph = UnitConverter('mi/hour')
+kilometers_per_hour = kph = UnitConverter('km/hour')
 kilogram = kilograms = kg = kgs = UnitConverter('kg')
 pound = pounds = lb = lbs = UnitConverter('lb')
 


Test patch:
diff --git a/stravalib/tests/integration/test_client.py b/stravalib/tests/integration/test_client.py
index 68bb295..cac34c0 100644
--- a/stravalib/tests/integration/test_client.py
+++ b/stravalib/tests/integration/test_client.py
@@ -6,6 +6,7 @@ import pytest
 import responses
 from responses import matchers
 
+import stravalib.unithelper as uh
 from stravalib.client import ActivityUploader
 from stravalib.exc import ActivityPhotoUploadFailed
 from stravalib.tests import RESOURCES_DIR
@@ -316,6 +317,19 @@ def test_get_activities(mock_strava_api, client, limit, n_raw_results, expected_
         assert activity_list[0].name == 'test_activity'
 
 
+def test_get_activities_quantity_addition(mock_strava_api, client):
+    mock_strava_api.get(
+        '/athlete/activities',
+        response_update={'distance': 1000.0},
+        n_results=2
+    )
+    act_list = list(client.get_activities(limit=2))
+    total_d = uh.meters(0)
+    total_d += act_list[0].distance
+    total_d += act_list[1].distance
+    assert total_d == uh.meters(2000.)
+
+
 def test_get_activities_paged(mock_strava_api, client):
     for i in range(1, 4):
         params = {'page': i, 'per_page': 200}
diff --git a/stravalib/tests/unit/test_unithelper.py b/stravalib/tests/unit/test_unithelper.py
index c5d736c..e896318 100644
--- a/stravalib/tests/unit/test_unithelper.py
+++ b/stravalib/tests/unit/test_unithelper.py
@@ -54,3 +54,15 @@ def test_is_quantity_type(obj, expected_result, expected_warning):
             assert uh.is_quantity_type(obj) == expected_result
     else:
         assert uh.is_quantity_type(obj) == expected_result
+
+
+def test_legacy_accessors():
+    assert uh.meters(10).num == 10
+    assert uh.meters(10).unit == 'meter'
+
+def test_arithmetic_comparison_support():
+    assert int(uh.meters(2.1)) == 2
+    assert float(uh.meters(2.1)) == 2.1
+    assert uh.meters(2) == uh.meters(2)
+    assert uh.meters(2) > uh.meters(1)
+    assert uh.meters(2) + uh.meters(1) == uh.meters(3)