Merge #953

953: Deprecate array protocol fallback r=hgrecco a=jthielen

This PR implements the changes discussed in #924: deprecating the `__array_*` attribute fallback and making `__array__` explicit in the Quantity API. Also, to enable the non-fallback behavior before the following minor version is released that removes the fallback, a check for a `PINT_ARRAY_PROTOCOL_FALLBACK` environment variable is added.

- [x] Closes #892, Closes #924
- [x] Executed ``black -t py36 . && isort -rc . && flake8`` with no errors
- [x] The change is fully covered by automated unit tests
- [x] Documented in docs/ as appropriate
- [x] Added an entry to the CHANGES file


Co-authored-by: Jon Thielen <github@jont.cc>

CHANGES

@@ -5,7 +5,12 @@ Pint Changelog
 -----------------
 
 - Switched test configuration to pytest and added tests of Pint's matplotlib support.
-  (Issue #954)
+  (Issue #954, Thanks Jon Thielen)
+- Deprecate array protocol fallback except where explicitly defined (`__array__`,
+  `__array_priority__`, `__array_function__`, `__array_ufunc__`). The fallback will
+  remain until the next minor version, or if the environment variable
+  `PINT_ARRAY_PROTOCOL_FALLBACK` is set to 0.
+  (Issue #953, Thanks Jon Thielen)
 - Removed eval usage when creating UnitDefinition and PrefixDefinition from string.
   (Issue #942)
 - Added `fmt_locale` argument to registry.

docs/numpy.rst

@@ -159,6 +159,15 @@ memory and CPU cycles. Therefore, for numerically intensive code, you
 might want to convert the objects first and then use directly the magnitude,
 such as by using Pint's `wraps` utility (see :ref:`wrapping`).
 
+Array interface protocol attributes (such as `__array_struct__` and
+`__array_interface__`) are available on Pint Quantities by deferring to the
+corresponding `__array_*` attribute on the magnitude as casted to an ndarray. This
+has been found to be potentially incorrect and to cause unexpected behavior, and has
+therefore been deprecated. As of the next minor version of Pint (or when the
+`PINT_ARRAY_PROTOCOL_FALLBACK` environment variable is set to 0 prior to importing
+Pint), attempting to access these attributes will instead raise an AttributeError.
+
+
 
 
 

pint/compat.py

@@ -7,6 +7,7 @@
     :copyright: 2013 by Pint Authors, see AUTHORS for more details.
     :license: BSD, see LICENSE for more details.
 """
+import os
 import tokenize
 from decimal import Decimal
 from io import BytesIO
@@ -94,6 +95,8 @@ def __array_function__(self, *args, **kwargs):
 
     NP_NO_VALUE = np._NoValue
 
+    ARRAY_FALLBACK = bool(int(os.environ.get("PINT_ARRAY_PROTOCOL_FALLBACK", 1)))
+
 except ImportError:
 
     np = None
@@ -107,9 +110,12 @@ class ndarray:
     HAS_NUMPY_ARRAY_FUNCTION = False
     SKIP_ARRAY_FUNCTION_CHANGE_WARNING = True
     NP_NO_VALUE = None
+    ARRAY_FALLBACK = False
 
     def _to_magnitude(value, force_ndarray=False):
-        if isinstance(value, (dict, bool)) or value is None:
+        if force_ndarray:
+            raise ValueError("Cannot force to ndarray when NumPy is not present.")
+        elif isinstance(value, (dict, bool)) or value is None:
             raise TypeError("Invalid magnitude for Quantity: {0!r}".format(value))
         elif isinstance(value, str) and value == "":
             raise ValueError("Quantity magnitude cannot be an empty string.")

pint/quantity.py

@@ -22,6 +22,7 @@
 
 from .compat import SKIP_ARRAY_FUNCTION_CHANGE_WARNING  # noqa: F401
 from .compat import (
+    ARRAY_FALLBACK,
     NUMPY_VER,
     BehaviorChangeWarning,
     _to_magnitude,
@@ -1454,6 +1455,14 @@ def _numpy_method_wrap(self, func, *args, **kwargs):
         else:
             return value
 
+    def __array__(self):
+        warnings.warn(
+            "The unit of the quantity is stripped when downcasting to ndarray.",
+            UnitStrippedWarning,
+            stacklevel=2,
+        )
+        return _to_magnitude(self._magnitude, force_ndarray=True)
+
     def clip(self, first=None, second=None, out=None, **kwargs):
         minimum = kwargs.get("min", first)
         maximum = kwargs.get("max", second)
@@ -1548,23 +1557,36 @@ def __len__(self):
         return len(self._magnitude)
 
     def __getattr__(self, item):
-        # Attributes starting with `__array_` are common attributes of NumPy ndarray.
-        # They are requested by numpy functions.
         if item.startswith("__array_"):
-            warnings.warn(
-                "The unit of the quantity is stripped when getting {} "
-                "attribute".format(item),
-                UnitStrippedWarning,
-                stacklevel=2,
-            )
-            if isinstance(self._magnitude, ndarray):
-                return getattr(self._magnitude, item)
+            # Handle array protocol attributes other than `__array__`
+            if ARRAY_FALLBACK:
+                # Deprecated fallback behavior
+                warnings.warn(
+                    (
+                        f"Array protocol attribute {item} accessed, with unit of the "
+                        "Quantity being stripped. This attribute will become unavailable "
+                        "in the next minor version of Pint. To make this potentially "
+                        "incorrect attribute unavailable now, set the "
+                        "PINT_ARRAY_PROTOCOL_FALLBACK environment variable to 0 before "
+                        "importing Pint."
+                    ),
+                    DeprecationWarning,
+                    stacklevel=2,
+                )
+
+                if isinstance(self._magnitude, ndarray):
+                    return getattr(self._magnitude, item)
+                else:
+                    # If an `__array_` attributes is requested but the magnitude is not an ndarray,
+                    # we convert the magnitude to a numpy ndarray.
+                    # TODO (#905 follow-up): Potentially problematic, investigate for duck arrays
+                    magnitude_as_array = _to_magnitude(
+                        self._magnitude, force_ndarray=True
+                    )
+                    return getattr(magnitude_as_array, item)
             else:
-                # If an `__array_` attributes is requested but the magnitude is not an ndarray,
-                # we convert the magnitude to a numpy ndarray.
-                # TODO (#905 follow-up): Potentially problematic, investigate for duck arrays
-                magnitude_as_array = _to_magnitude(self._magnitude, force_ndarray=True)
-                return getattr(magnitude_as_array, item)
+                # TODO (next minor version): ARRAY_FALLBACK is removed and this becomes the standard behavior
+                raise AttributeError(f"Array protocol attribute {item} not available.")
         elif item in HANDLED_UFUNCS or item in self._wrapped_numpy_methods:
             # TODO (#905 follow-up): Potentially problematic, investigate for duck arrays/scalars
             magnitude_as_array = _to_magnitude(self._magnitude, True)

pint/testsuite/test_numpy.py

@@ -1,8 +1,9 @@
 import copy
 import operator as op
 import unittest
+from unittest.mock import patch
 
-from pint import DimensionalityError, OffsetUnitCalculusError
+from pint import DimensionalityError, OffsetUnitCalculusError, UnitStrippedWarning
 from pint.compat import np
 from pint.testsuite import QuantityTestCase, helpers
 from pint.testsuite.test_umath import TestUFuncs
@@ -1004,6 +1005,26 @@ def test_insert(self):
             np.array([[1, 0, 2], [3, 0, 4]]) * self.ureg.m,
         )
 
+    @patch("pint.quantity.ARRAY_FALLBACK", False)
+    def test_ndarray_downcast(self):
+        with self.assertWarns(UnitStrippedWarning):
+            np.asarray(self.q)
+
+    def test_array_protocol_fallback(self):
+        with self.assertWarns(DeprecationWarning) as cm:
+            for attr in ("__array_struct__", "__array_interface__"):
+                getattr(self.q, attr)
+                warning_text = str(cm.warnings[0].message)
+                self.assertTrue(
+                    f"unit of the Quantity being stripped" in warning_text
+                    and "will become unavailable" in warning_text
+                )
+
+    @patch("pint.quantity.ARRAY_FALLBACK", False)
+    def test_array_protocol_unavailable(self):
+        for attr in ("__array_struct__", "__array_interface__"):
+            self.assertRaises(AttributeError, getattr, self.q, attr)
+
 
 @unittest.skip
 class TestBitTwiddlingUfuncs(TestUFuncs):

pint/testsuite/test_quantity.py

@@ -532,6 +532,10 @@ def test_array_function_warning_on_creation(self):
             warnings.filterwarnings("error")
             self.Q_([])
 
+    @helpers.requires_not_numpy()
+    def test_no_ndarray_coercion_without_numpy(self):
+        self.assertRaises(ValueError, self.Q_(1, "m").__array__)
+
 
 class TestQuantityToCompact(QuantityTestCase):
     def assertQuantityAlmostIdentical(self, q1, q2):