From c6066618d4fd11cff69464ce3c21e9461d85d3a7 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 23 Apr 2018 06:04:12 -0500 Subject: [PATCH 1/3] TST: Added warning test about values --- pandas/tests/extension/base/interface.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pandas/tests/extension/base/interface.py b/pandas/tests/extension/base/interface.py index 2162552e9650d..f52bcf51c2caf 100644 --- a/pandas/tests/extension/base/interface.py +++ b/pandas/tests/extension/base/interface.py @@ -50,3 +50,10 @@ def test_is_extension_array_dtype(self, data): assert is_extension_array_dtype(data.dtype) assert is_extension_array_dtype(pd.Series(data)) assert isinstance(data.dtype, ExtensionDtype) + + def test_no_values_attribute(self, data): + if hasattr(data, 'values') and not hasattr(data.values, 'dtype'): + msg = ("ExtensionArray contains a 'values' attribute that does " + "not have a dtype attribute. This may cause issues in " + "pandas' internals.") + raise ValueError(msg) From 559d0ea5e85356db78f45e949707a7b9a815ff00 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 23 Apr 2018 06:36:16 -0500 Subject: [PATCH 2/3] Added explanation --- pandas/tests/extension/base/interface.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pandas/tests/extension/base/interface.py b/pandas/tests/extension/base/interface.py index f52bcf51c2caf..8664cd311a859 100644 --- a/pandas/tests/extension/base/interface.py +++ b/pandas/tests/extension/base/interface.py @@ -52,6 +52,11 @@ def test_is_extension_array_dtype(self, data): assert isinstance(data.dtype, ExtensionDtype) def test_no_values_attribute(self, data): + # GH-20735 + # Currently, pandas has places where we accepts Union[ndarray, EA] + # but in practice expect an ndarray, since we get `data.values.dtype`. + # This warns users to avoid using a `.values` attribute that is not + # an ndarray-like if hasattr(data, 'values') and not hasattr(data.values, 'dtype'): msg = ("ExtensionArray contains a 'values' attribute that does " "not have a dtype attribute. This may cause issues in " From e5ee4e9b64bae064f3afa1b8045c948aa380082b Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 23 Apr 2018 06:50:53 -0500 Subject: [PATCH 3/3] Added notes, tests --- pandas/core/arrays/base.py | 13 +++++++++++++ pandas/tests/extension/test_values.py | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 pandas/tests/extension/test_values.py diff --git a/pandas/core/arrays/base.py b/pandas/core/arrays/base.py index 97a764fa7dbe8..0e1c643d58596 100644 --- a/pandas/core/arrays/base.py +++ b/pandas/core/arrays/base.py @@ -5,6 +5,19 @@ This is an experimental API and subject to breaking changes without warning. """ +# Some notes, just for implementors. +# +# * Avoid a `.values` attribute +# There are corners in pandas that assume assume array.values is an ndarray, +# and does ndarray things like ge the dtype. If you have a non-ndarray +# `.values` attribute, things may break. We're working to fix these. +# * Reference the default implementations, but avoid using private methods. +# Some default implementations (e.g. `factorize`) use pandas' internal +# methods. If possible, avoid using those methods when overriding the default +# implementation. If that isn't possible, raise an issue to discuss adding +# those to the public API. + + import numpy as np from pandas.errors import AbstractMethodError diff --git a/pandas/tests/extension/test_values.py b/pandas/tests/extension/test_values.py new file mode 100644 index 0000000000000..754be455a6ed9 --- /dev/null +++ b/pandas/tests/extension/test_values.py @@ -0,0 +1,23 @@ +import pytest + +from pandas.tests.extension.base import BaseInterfaceTests +from pandas.tests.extension.json.array import JSONArray + + +class JSONArray2(JSONArray): + @property + def values(self): + return self.data + + +@pytest.fixture +def data(): + return JSONArray2([{"A": [1, 2], "B": [3, 4]}]) + + +class TestBaseInterfaceTests(BaseInterfaceTests): + def test_no_values_attribute(self, data): + # GH-20735 + with pytest.raises(ValueError) as m: + super(TestBaseInterfaceTests, self).test_no_values_attribute(data) + assert m.match("ExtensionArray contains")