Fix _is_tensorflow_array. (matplotlib#30114)

The previous implementation was clearly wrong (the isinstance check
would raise TypeError as the second argument would be a bool), but the
tests didn't catch that because the bug led to _is_tensorflow_array
returning False, then _unpack_to_numpy returning the original input,
and then assert_array_equal implicitly converting `result` by calling
`__array__` on it.  Fix the test by explicitly checking that `result`
is indeed a numpy array, and also fix _is_tensorflow_array with more
restrictive exception catching (also applied to _is_torch_array,
_is_jax_array, and _is_pandas_dataframe, while we're at it).

Author: anntzer

lib/matplotlib/cbook.py

@@ -2331,42 +2331,56 @@ def _picklable_class_constructor(mixin_class, fmt, attr_name, base_class):
 
 
 def _is_torch_array(x):
-    """Check if 'x' is a PyTorch Tensor."""
+    """Return whether *x* is a PyTorch Tensor."""
     try:
-        # we're intentionally not attempting to import torch. If somebody
-        # has created a torch array, torch should already be in sys.modules
-        return isinstance(x, sys.modules['torch'].Tensor)
-    except Exception:  # TypeError, KeyError, AttributeError, maybe others?
-        # we're attempting to access attributes on imported modules which
-        # may have arbitrary user code, so we deliberately catch all exceptions
-        return False
+        # We're intentionally not attempting to import torch. If somebody
+        # has created a torch array, torch should already be in sys.modules.
+        tp = sys.modules.get("torch").Tensor
+    except AttributeError:
+        return False  # Module not imported or a nonstandard module with no Tensor attr.
+    return (isinstance(tp, type)  # Just in case it's a very nonstandard module.
+            and isinstance(x, tp))
 
 
 def _is_jax_array(x):
-    """Check if 'x' is a JAX Array."""
+    """Return whether *x* is a JAX Array."""
     try:
-        # we're intentionally not attempting to import jax. If somebody
-        # has created a jax array, jax should already be in sys.modules
-        return isinstance(x, sys.modules['jax'].Array)
-    except Exception:  # TypeError, KeyError, AttributeError, maybe others?
-        # we're attempting to access attributes on imported modules which
-        # may have arbitrary user code, so we deliberately catch all exceptions
-        return False
+        # We're intentionally not attempting to import jax. If somebody
+        # has created a jax array, jax should already be in sys.modules.
+        tp = sys.modules.get("jax").Array
+    except AttributeError:
+        return False  # Module not imported or a nonstandard module with no Array attr.
+    return (isinstance(tp, type)  # Just in case it's a very nonstandard module.
+            and isinstance(x, tp))
+
+
+def _is_pandas_dataframe(x):
+    """Check if *x* is a Pandas DataFrame."""
+    try:
+        # We're intentionally not attempting to import Pandas. If somebody
+        # has created a Pandas DataFrame, Pandas should already be in sys.modules.
+        tp = sys.modules.get("pandas").DataFrame
+    except AttributeError:
+        return False  # Module not imported or a nonstandard module with no Array attr.
+    return (isinstance(tp, type)  # Just in case it's a very nonstandard module.
+            and isinstance(x, tp))
 
 
 def _is_tensorflow_array(x):
-    """Check if 'x' is a TensorFlow Tensor or Variable."""
+    """Return whether *x* is a TensorFlow Tensor or Variable."""
     try:
-        # we're intentionally not attempting to import TensorFlow. If somebody
-        # has created a TensorFlow array, TensorFlow should already be in sys.modules
-        # we use `is_tensor` to not depend on the class structure of TensorFlow
-        # arrays, as `tf.Variables` are not instances of `tf.Tensor`
-        # (they both convert the same way)
-        return isinstance(x, sys.modules['tensorflow'].is_tensor(x))
-    except Exception:  # TypeError, KeyError, AttributeError, maybe others?
-        # we're attempting to access attributes on imported modules which
-        # may have arbitrary user code, so we deliberately catch all exceptions
+        # We're intentionally not attempting to import TensorFlow. If somebody
+        # has created a TensorFlow array, TensorFlow should already be in
+        # sys.modules we use `is_tensor` to not depend on the class structure
+        # of TensorFlow arrays, as `tf.Variables` are not instances of
+        # `tf.Tensor` (they both convert the same way).
+        is_tensor = sys.modules.get("tensorflow").is_tensor
+    except AttributeError:
         return False
+    try:
+        return is_tensor(x)
+    except Exception:
+        return False  # Just in case it's a very nonstandard module.
 
 
 def _unpack_to_numpy(x):
@@ -2421,15 +2435,3 @@ def _auto_format_str(fmt, value):
         return fmt % (value,)
     except (TypeError, ValueError):
         return fmt.format(value)
-
-
-def _is_pandas_dataframe(x):
-    """Check if 'x' is a Pandas DataFrame."""
-    try:
-        # we're intentionally not attempting to import Pandas. If somebody
-        # has created a Pandas DataFrame, Pandas should already be in sys.modules
-        return isinstance(x, sys.modules['pandas'].DataFrame)
-    except Exception:  # TypeError, KeyError, AttributeError, maybe others?
-        # we're attempting to access attributes on imported modules which
-        # may have arbitrary user code, so we deliberately catch all exceptions
-        return False

lib/matplotlib/tests/test_cbook.py

@@ -1000,6 +1000,7 @@ def __array__(self):
     torch_tensor = torch.Tensor(data)
 
     result = cbook._unpack_to_numpy(torch_tensor)
+    assert isinstance(result, np.ndarray)
     # compare results, do not check for identity: the latter would fail
     # if not mocked, and the implementation does not guarantee it
     # is the same Python object, just the same values.
@@ -1028,6 +1029,7 @@ def __array__(self):
     jax_array = jax.Array(data)
 
     result = cbook._unpack_to_numpy(jax_array)
+    assert isinstance(result, np.ndarray)
     # compare results, do not check for identity: the latter would fail
     # if not mocked, and the implementation does not guarantee it
     # is the same Python object, just the same values.
@@ -1057,6 +1059,7 @@ def __array__(self):
     tf_tensor = tensorflow.Tensor(data)
 
     result = cbook._unpack_to_numpy(tf_tensor)
+    assert isinstance(result, np.ndarray)
     # compare results, do not check for identity: the latter would fail
     # if not mocked, and the implementation does not guarantee it
     # is the same Python object, just the same values.