Repository: pandas-dev/pandas
Pull request: Interval dtype fix
- [x] closes #25283
- [x] test added to ensure pd.api.types.IntervalDtype('IntervalA') throws TypeError
- [x] test added to ensure pd.api.types.IntervalDtype.construct_from_string('IntervalA') throws TypeError
- [x] test added to ensure pd.api.types.IntervalDtype.is_dtype('IntervalA') returns false
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] test added to ensure * `Series(['IntervalA', 'IntervalB', 'IntervalC']) == 'IntervalA'` returns the expected boolean series
- [x] what's new note added to doc/source/whatsnew/v0.24.2.rst documenting regression fix

Problem statement:
Pandas dataFrame - selecting data - 'Interval'  Recursion error 
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd
df = pd.DataFrame({'SPCName': ['IntervalXxxxxxxxxxxxxxxxxxxxxxxxx']})
df[df.SPCName == 'IntervalA']
```
#### Problem description

I am getting a recursion error from the above code ( RuntimeError: maximum recursion depth exceeded while calling a Python object )

This issue did not occur in version 0.23.4 but occurs when upgrading to 0.24.1.

I am not aware why using 'IntervalA' in a Boolean lookup or when using the loc indexer should result in RuntimeError

This issue appears to occur regardless of what follows the str 'Interval' for example 'IntervalABC' also fails.

#### Full traceback
python2.7/site-packages/pandas/core/ops.pyc in wrapper(self, other, axis)
   1726 
   1727         elif (is_extension_array_dtype(self) or
-> 1728               (is_extension_array_dtype(other) and not is_scalar(other))):
   1729             # Note: the `not is_scalar(other)` condition rules out
   1730             # e.g. other == "category"

python2.7/site-packages/pandas/core/dtypes/common.pyc in is_extension_array_dtype(arr_or_dtype)
   1747     dtype = getattr(arr_or_dtype, 'dtype', arr_or_dtype)
   1748     return (isinstance(dtype, ExtensionDtype) or
-> 1749             registry.find(dtype) is not None)
   1750 
   1751 

python2.7/site-packages/pandas/core/dtypes/dtypes.pyc in find(self, dtype)
     87         for dtype_type in self.dtypes:
     88             try:
---> 89                 return dtype_type.construct_from_string(dtype)
     90             except TypeError:
     91                 pass

python2.7/site-packages/pandas/core/dtypes/dtypes.pyc in construct_from_string(cls, string)
    936             (string.startswith('interval') or
    937              string.startswith('Interval'))):
--> 938             return cls(string)
    939 
    940         msg = "a string needs to be passed, got type {typ}"

python2.7/site-packages/pandas/core/dtypes/dtypes.pyc in __new__(cls, subtype)
    897 
    898             try:
--> 899                 subtype = pandas_dtype(subtype)
    900             except TypeError:
    901                 raise TypeError("could not construct IntervalDtype")

python2.7/site-packages/pandas/core/dtypes/common.pyc in pandas_dtype(dtype)
   2002 
   2003     # registered extension types
-> 2004     result = registry.find(dtype)
   2005     if result is not None:
   2006         return result

... last 4 frames repeated, from the frame below ...

python2.7/site-packages/pandas/core/dtypes/dtypes.pyc in find(self, dtype)
     87         for dtype_type in self.dtypes:
     88             try:
---> 89                 return dtype_type.construct_from_string(dtype)
     90             except TypeError:
     91                 pass

#### Expected Output
Empty dataframe because the str 'IntervalA' doesnt exist in the dataframe col SPCName.

#### Output of ``pd.show_versions()``

<details>

[paste the output of ``pd.show_versions()`` here below this line]
INSTALLED VERSIONS
------------------
commit: None
python: 2.7.14.final.0
python-bits: 64
OS: Linux
OS-release: 2.6.32-642.6.2.el6.x86_64
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US
LOCALE: None.None

pandas: 0.24.1
pytest: 3.6.4
pip: 18.0
setuptools: 40.2.0
Cython: None
numpy: 1.15.1
scipy: None
pyarrow: None
xarray: None
IPython: 5.8.0
sphinx: None
patsy: None
dateutil: 2.7.3
pytz: 2018.5
blosc: None
bottleneck: None
tables: 3.4.4
numexpr: 2.6.8
feather: None
matplotlib: 2.2.3
openpyxl: 2.5.12
xlrd: None
xlwt: None
xlsxwriter: None
lxml.etree: None
bs4: None
html5lib: 1.0.1
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
gcsfs: None
</details>

Hints from issue discussion:
Thanks, the cause the error looks to be in `IntervalDtype` when a close but invalid string is passed:
```python
In [1]: import pandas as pd; pd.__version__
Out[1]: '0.25.0.dev0+86.g1d1b14c7e'

In [2]: pd.api.types.IntervalDtype('IntervalA')
---------------------------------------------------------------------------
RecursionError: maximum recursion depth exceeded in __instancecheck__

In [3]: pd.api.types.IntervalDtype.construct_from_string('IntervalA')
---------------------------------------------------------------------------
RecursionError: maximum recursion depth exceeded while calling a Python object

In [4]: pd.api.types.IntervalDtype.is_dtype('IntervalA')
---------------------------------------------------------------------------
RecursionError: maximum recursion depth exceeded
```
The correct output is: `[2]` and `[3]` should raise, and `[4]` should return `False`.

The fix is to more strongly enforce what the `IntervalDtype` looks for when parsing strings.  The valid string representations for an `IntervalDtype` are `'interval'` or `'interval[<subtype>]'` (e.g. `'interval[int64]'`) with the leading 'i' optionally being capitalized.

The `RecursionError` in `IntervalDtype` itself is a bug that was present in 0.23.4 and it looks like some changes in 0.24.0 surfaced this bug in the code responsible for filtering.

Commit message: IntervalDtype construct from string test error message handling updated, regex match in error message string was causing test to fail

Patch to review:
diff --git a/pandas/core/dtypes/dtypes.py b/pandas/core/dtypes/dtypes.py
index 640d43f3b0e03..e78a543d7faa7 100644
--- a/pandas/core/dtypes/dtypes.py
+++ b/pandas/core/dtypes/dtypes.py
@@ -931,10 +931,15 @@ def construct_from_string(cls, string):
         attempt to construct this type from a string, raise a TypeError
         if its not possible
         """
-        if (isinstance(string, compat.string_types) and
-            (string.startswith('interval') or
-             string.startswith('Interval'))):
-            return cls(string)
+        if isinstance(string, compat.string_types):
+            if (string.lower() == 'interval' or
+               cls._match.search(string) is not None):
+                    return cls(string)
+            else:
+                msg = ('Incorrectly formatted string passed to constructor. '
+                       'Valid formats include Interval or Inverval[dtype] '
+                       'where dtype is numeric, datetime, or timedelta')
+                raise TypeError(msg)
 
         msg = "a string needs to be passed, got type {typ}"
         raise TypeError(msg.format(typ=type(string)))
@@ -978,7 +983,7 @@ def is_dtype(cls, dtype):
                         return True
                     else:
                         return False
-                except ValueError:
+                except (ValueError, TypeError):
                     return False
             else:
                 return False
diff --git a/pandas/tests/dtypes/test_dtypes.py b/pandas/tests/dtypes/test_dtypes.py
index 710f215686eab..826f7ae26a797 100644
--- a/pandas/tests/dtypes/test_dtypes.py
+++ b/pandas/tests/dtypes/test_dtypes.py
@@ -511,10 +511,11 @@ def test_construction_not_supported(self, subtype):
         with pytest.raises(TypeError, match=msg):
             IntervalDtype(subtype)
 
-    def test_construction_errors(self):
+    @pytest.mark.parametrize('subtype', ['xx', 'IntervalA', 'Interval[foo]'])
+    def test_construction_errors(self, subtype):
         msg = 'could not construct IntervalDtype'
         with pytest.raises(TypeError, match=msg):
-            IntervalDtype('xx')
+            IntervalDtype(subtype)
 
     def test_construction_from_string(self):
         result = IntervalDtype('interval[int64]')
@@ -523,7 +524,7 @@ def test_construction_from_string(self):
         assert is_dtype_equal(self.dtype, result)
 
     @pytest.mark.parametrize('string', [
-        'foo', 'foo[int64]', 0, 3.14, ('a', 'b'), None])
+        0, 3.14, ('a', 'b'), None])
     def test_construction_from_string_errors(self, string):
         # these are invalid entirely
         msg = 'a string needs to be passed, got type'
@@ -532,13 +533,16 @@ def test_construction_from_string_errors(self, string):
             IntervalDtype.construct_from_string(string)
 
     @pytest.mark.parametrize('string', [
-        'interval[foo]'])
+        'foo', 'foo[int64]'])
     def test_construction_from_string_error_subtype(self, string):
         # this is an invalid subtype
-        msg = 'could not construct IntervalDtype'
+        msg = ('Incorrectly formatted string passed to constructor. '
+               'Valid formats include Interval or Inverval[dtype] '
+               'where dtype is numeric, datetime, or timedelta')
 
-        with pytest.raises(TypeError, match=msg):
+        with pytest.raises(TypeError) as e:
             IntervalDtype.construct_from_string(string)
+            assert e == msg
 
     def test_subclass(self):
         a = IntervalDtype('interval[int64]')
@@ -559,6 +563,7 @@ def test_is_dtype(self):
         assert not IntervalDtype.is_dtype('U')
         assert not IntervalDtype.is_dtype('S')
         assert not IntervalDtype.is_dtype('foo')
+        assert not IntervalDtype.is_dtype('IntervalA')
         assert not IntervalDtype.is_dtype(np.object_)
         assert not IntervalDtype.is_dtype(np.int64)
         assert not IntervalDtype.is_dtype(np.float64)


Code review comments:
diff_hunk: @@ -931,10 +931,15 @@ def construct_from_string(cls, string):
         attempt to construct this type from a string, raise a TypeError
         if its not possible
         """
-        if (isinstance(string, compat.string_types) and
-            (string.startswith('interval') or
-             string.startswith('Interval'))):
-            return cls(string)
+        if isinstance(string, compat.string_types):
+            if (string.lower() == 'interval' or
+               cls._match.search(string) is not None):
+                    return cls(string)
+            else:
+                msg = ('Incorrectly formatted string passed to constructor. '
path: pandas/core/dtypes/dtypes.py
text: @user1:
you don't actually need the else here

@author:
the else statement is now removed. thanks for the help! 

cases from the original post are covered in pandas/core/tests/dtypes/test_dtypes.py

pd.api.types.IntervalDtype('IntervalA') covered in line 514
pd.api.types.IntervalDtype.construct_from_string('IntervalA') is covered indirectly but got lost in the shuffle, I will re-add this one explicitly 
pd.api.types.IntervalDtype.is_dtype('IntervalA') covered in line 565

@author:
pd.api.types.IntervalDtype.construct_from_string('IntervalA') now covered in line 536

diff_hunk: @@ -931,10 +931,15 @@ def construct_from_string(cls, string):
         attempt to construct this type from a string, raise a TypeError
         if its not possible
         """
-        if (isinstance(string, compat.string_types) and
-            (string.startswith('interval') or
-             string.startswith('Interval'))):
-            return cls(string)
+        if isinstance(string, compat.string_types):
+            if (string.lower() == 'interval' or
+               cls._match.search(string) is not None):
+                    return cls(string)
+            else:
+                msg = ('Incorrectly formatted string passed to constructor. '
+                       'Valid formats include Interval or Inverval[dtype] '
path: pandas/core/dtypes/dtypes.py
text: @user1:
Inverval[dtype] --> Interval[dtype]

@author:
good catch! 
sorry missed this in my previous push, should be all set now.

diff_hunk: @@ -532,13 +533,16 @@ def test_construction_from_string_errors(self, string):
             IntervalDtype.construct_from_string(string)
 
     @pytest.mark.parametrize('string', [
-        'interval[foo]'])
+        'foo', 'foo[int64]'])
     def test_construction_from_string_error_subtype(self, string):
         # this is an invalid subtype
-        msg = 'could not construct IntervalDtype'
+        msg = ('Incorrectly formatted string passed to constructor. '
+               'Valid formats include Interval or Inverval[dtype] '
+               'where dtype is numeric, datetime, or timedelta')
 
-        with pytest.raises(TypeError, match=msg):
path: pandas/tests/dtypes/test_dtypes.py
text: @user1:
keep using `match`; you'll just need to escape the square brackets in the message since `pytest` is interpreting it as regex, i.e. `'Interval\[dtype\]'`

@author:
Working on this now, running into some PEP8 issues, saying 'escape sequence is invalid for \[ and \]'. Just need to clear these warnings up

diff_hunk: @@ -931,10 +931,15 @@ def construct_from_string(cls, string):
         attempt to construct this type from a string, raise a TypeError
         if its not possible
         """
-        if (isinstance(string, compat.string_types) and
-            (string.startswith('interval') or
-             string.startswith('Interval'))):
-            return cls(string)
+        if isinstance(string, compat.string_types):
+            if (string.lower() == 'interval' or
+               cls._match.search(string) is not None):
+                    return cls(string)
+            else:
+                msg = ('Incorrectly formatted string passed to constructor. '
+                       'Valid formats include Interval or Inverval[dtype] '
+                       'where dtype is numeric, datetime, or timedelta')
+                raise TypeError(msg)
 
         msg = "a string needs to be passed, got type {typ}"
         raise TypeError(msg.format(typ=type(string)))
path: pandas/core/dtypes/dtypes.py
text: @user2:
I like the changes here; seems like it'd be a little cleaner to swap around the checks a bit to remove an indentation level but keep the code itself basically identical:
```python
@user1
def construct_from_string(cls, string):
    """
    attempt to construct this type from a string, raise a TypeError
    if its not possible
    """
    if not isinstance(string, compat.string_types):
        msg = "a string needs to be passed, got type {typ}"
        raise TypeError(msg.format(typ=type(string)))

    if (string.lower() == 'interval' or
            cls._match.search(string) is not None):
        return cls(string)

    msg = ('Incorrectly formatted string passed to constructor. '
           'Valid formats include Interval or Interval[dtype] '
           'where dtype is numeric, datetime, or timedelta')
    raise TypeError(msg)
```

@author:
Yep good suggestion. This is much cleaner. The order of logic is updated now