Skip to content

BUG: na_value = None in ExtensionDtype causes ExtensionArray tests to fail #44602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
3 tasks done
venaturum opened this issue Nov 24, 2021 · 4 comments
Open
3 tasks done
Labels
Bug ExtensionArray Extending pandas with custom dtypes or arrays. Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments

@venaturum
Copy link
Contributor

venaturum commented Nov 24, 2021

  • I have checked that this issue has not already been reported.

  • I have confirmed this bug exists on the latest version of pandas.

  • I have confirmed this bug exists on the master branch of pandas.

Reproducible Example

import numpy as np
import pandas as pd
from pandas.testing import assert_series_equal
import numbers
from collections.abc import Iterable
from pandas.api.extensions import (
    ExtensionArray,
    ExtensionDtype,
    register_extension_dtype,
)

def test_empty_series_construction_with_na_value_None():

    class DummyClass:

        def __init__(self):
            pass

    @register_extension_dtype
    class DummyDtype(ExtensionDtype):

        type = DummyClass
        name = "dummy"
        na_value = None

        @classmethod
        def construct_from_string(cls, string):
            if string == cls.name:
                return cls()
            else:
                raise TypeError("Cannot construct a '{}' from " "'{}'".format(cls, string))

        @classmethod
        def construct_array_type(cls):
            return DummyArray


    class DummyArray(ExtensionArray):

        _dtype = DummyDtype()

        def __init__(self, values):
            self._values = values

        @property
        def dtype(self):
            return self._dtype

        def __len__(self):
            return len(self._values)

        def __getitem__(self, idx):
            if isinstance(idx, numbers.Integral):
                return self._values[idx]
            elif isinstance(idx, np.ndarray) and idx.dtype.kind == "b" and idx.shape == self.shape:
                indexer= idx.nonzero()[0]
                return self[indexer]
            elif isinstance(idx, Iterable):
                new_values = [self._values[i] for i in idx]
                return DummyArray(new_values)
            elif isinstance(idx, slice):
                return DummyArray(self._values[idx])
            else:
                raise TypeError("Index type not supported", idx)

        @classmethod
        def _from_sequence(cls, scalars, dtype=None, copy=False):
            if isinstance(scalars, DummyClass):
                scalars = [scalars]
            return DummyArray(scalars)

        def take(
            self,
            indices,
            *,
            allow_fill: bool = False,
            fill_value = None,
        ):
            if allow_fill:
                scalars = [self[i] if i != -1 else fill_value for i in indices]
            else:
                scalars = [self[i] for i in indices]
            return type(self)._from_sequence(scalars)

        def isna(self):
            return np.array([x is None for x in self._values], dtype=bool)


    result = pd.Series(index=[1, 2, 3], dtype="dummy")
    expected = pd.Series([None] * 3, index=[1, 2, 3], dtype="dummy")
    assert_series_equal(result, expected)

test_empty_series_construction_with_na_value_None()

This results in a recursion error

Issue Description

There is a test "test_series_constructor_no_data_with_index" in pandas.tests.extension.base.constructors.py which tests the following

result = pd.Series(index=[1, 2, 3], dtype=dtype)
expected = pd.Series([na_value] * 3, index=[1, 2, 3], dtype=dtype)
self.assert_series_equal(result, expected)

The test does not pass for ExtensionDtypes which have a na_value set to None.

Expected Behavior

The result of pd.Series(index=[1, 2, 3], dtype="dummy") should be

1   None
2   None
3   None
dtype: dummy

Installed Versions

INSTALLED VERSIONS

commit : 945c9ed
python : 3.7.5.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19041
machine : AMD64
processor : Intel64 Family 6 Model 158 Stepping 10, GenuineIntel
byteorder : little
LC_ALL : None
LANG : None
LOCALE : None.None

pandas : 1.3.4
numpy : 1.21.4
pytz : 2021.3
dateutil : 2.8.2
pip : 19.2.3
setuptools : 41.2.0
Cython : None
pytest : None
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : None
pandas_datareader: None
bs4 : None
bottleneck : None
fsspec : None
fastparquet : None
gcsfs : None
matplotlib : None
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pyxlsb : None
s3fs : None
scipy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
numba : None

@venaturum venaturum added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Nov 24, 2021
@mroeschke mroeschke added ExtensionArray Extending pandas with custom dtypes or arrays. Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Dec 27, 2021
@jbrockmendel
Copy link
Member

I had to update the OP to get the test to run on main. In particular I added basic isna and take methods, adjusted __getitem__ to support boolean mask and ndarray[int].

With these updates, the OP example now works as expected for me on main. @jorisvandenbossche is the problem with geopandas (motivating #54930) distinct from whatever is/was going on here?

@jorisvandenbossche
Copy link
Member

I assume this is indeed fixed (as a side effect of other changes) in the latest version of pandas. When opening the other PR I did test it on the geopandas side to change the value of GeometryDtype.na_value, and it does seem to work with the latest pandas.

@jorisvandenbossche
Copy link
Member

We might want to change one of our existing test EAs to use None as the na_value to properly test this on our side. For example the DateDtype (although that is currently only used for a specific json test, so we would need to add all the base extension tests for this one, so not ideal. But other test EAs like json and list are also not ideal because being nested also brings in some other corner cases / skips; And decimal already uses Decimal("NaN"))

@jbrockmendel
Copy link
Member

We might want to change one of our existing test EAs to use None as the na_value to properly test this on our side

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug ExtensionArray Extending pandas with custom dtypes or arrays. Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants