Repository: pandas-dev/pandas
Pull request: REGR: allow merging on object boolean columns
Fixes https://github.com/pandas-dev/pandas/issues/21119

This is a quick/hacky fix for the specific case in that issue, i.e. merging boolean / object boolean columns. 

However, I have the feeling we should do a more general solution for cases where object dtyped columns might be actually mergeable. The problem is that inferring the type can be costly? (and eg when having actual strings object columns, this inferring was also unnecessary and thus will unneeded slow down `merge`?)

Problem statement:
pandas 0.23 fails on column<->index boolean merge
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd

df1 = pd.DataFrame({'x': pd.Series([True, False], dtype=bool)})
df2 = df1.set_index('x')

df1.merge(df2, left_on='x', right_index=True)
```

#### Problem description
On Pandas `0.23`, I get the following Exception:

```
venv/local/lib/python2.7/site-packages/pandas/core/frame.pyc in merge(self, right, how, on, left_on, right_on, left_index, right_index, sort, suffixes, copy, indicator, validate)
   6377                      right_on=right_on, left_index=left_index,
   6378                      right_index=right_index, sort=sort, suffixes=suffixes,
-> 6379                      copy=copy, indicator=indicator, validate=validate)
   6380
   6381     def round(self, decimals=0, *args, **kwargs):

venv/local/lib/python2.7/site-packages/pandas/core/reshape/merge.pyc in merge(left, right, how, on, left_on, right_on, left_index, right_index, sort, suffixes, copy, indicator, validate)
     58                          right_index=right_index, sort=sort, suffixes=suffixes,
     59                          copy=copy, indicator=indicator,
---> 60                          validate=validate)
     61     return op.get_result()
     62

venv/local/lib/python2.7/site-packages/pandas/core/reshape/merge.pyc in __init__(self, left, right, how, on, left_on, right_on, axis, left_index, right_index, sort, suffixes, copy, indicator, validate)
    552         # validate the merge keys dtypes. We may need to coerce
    553         # to avoid incompat dtypes
--> 554         self._maybe_coerce_merge_keys()
    555
    556         # If argument passed to validate,

venv/local/lib/python2.7/site-packages/pandas/core/reshape/merge.pyc in _maybe_coerce_merge_keys(self)
    976             # incompatible dtypes GH 9780, GH 15800
    977             elif is_numeric_dtype(lk) and not is_numeric_dtype(rk):
--> 978                 raise ValueError(msg)
    979             elif not is_numeric_dtype(lk) and is_numeric_dtype(rk):
    980                 raise ValueError(msg)

ValueError: You are trying to merge on bool and object columns. If you wish to proceed you should use pd.concat
```

#### Expected Output

```
       x
0   True
1  False
```

#### Output of ``pd.show_versions()``

<details>

```
INSTALLED VERSIONS
------------------
commit: None
python: 2.7.13.final.0
python-bits: 64
OS: Linux
OS-release: 4.9.49-moby
machine: x86_64
processor:
byteorder: little
LC_ALL: en_US.UTF-8
LANG: en_US.UTF-8
LOCALE: None.None

pandas: 0.23.0
pytest: 3.4.0
pip: 9.0.3
setuptools: 39.0.1
Cython: 0.28.1
numpy: 1.14.0
scipy: 1.0.0
pyarrow: None
xarray: None
IPython: 5.5.0
sphinx: 1.6.5
patsy: 0.4.1
dateutil: 2.6.1
pytz: 2017.3
blosc: None
bottleneck: 1.2.1
tables: None
numexpr: 2.6.4
feather: None
matplotlib: 2.1.0
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.8.1
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
```
</details>

Hints from issue discussion:
@crepererum : Thanks for reporting this!  Was this working on a previous version by any chance?
Yes, the expected output was produced by `0.22.0`.

Does pandas, by any chance, have some infrastructure for bisecting the commit / merge that introduced the issue? 
`git bisect` is what you're looking for.  That's a pure `git` construct BTW.
I know GIT does the job, I just thought there may be already be some scripts to simplify the build + test process or even an archive of pre built wheels for every master commit. Anyway, no worries, I'll see what if I find the the commit this week. 
```
1355df64af894836f86cbc75b0c3b1435331aeb4 is the first bad commit
commit 1355df64af894836f86cbc75b0c3b1435331aeb4
Author: Paul Reidy <paul_reidy@outlook.com>
Date:   Sun Dec 10 15:41:36 2017 +0000

    ERR: ValueError when merging on incompatible dtypes (#18674)
```
The problem here is: the check introduced is in this commit does the correct thing. The index dtype is wrong (it's `object`, not `bool`), which can also be shown be this simple example (identical result for `0.22.0` and `0.23.0`):
```
>>> pd.Index([True, False], dtype=bool)
Index([True, False], dtype='object')
```

Or in other words: the index dtype is wrong in both versions, the check that was introduced in-between just makes the problem visible.
@crepererum Thanks for figuring that out. 
So the underlying cause (which now comes up due to that change), is that boolean values in the index are stored as object array (we don't have a BooleanIndex), so it things the types are not compatible (object vs bool). 

So we will have to 'infer' the dtype of the index to cover those cases.
Yes, fully correct, we've come to the same conclusion :-) 
The index dtype being object is not really "wrong", it is just how boolean values are currently stored in indexes. So the code to check if the dtypes are compatible in the merging code, needs to be made aware of this fact.
BTW: the problem is NOT specific to indices. This code also doesn't work:

```
import pandas as pd

df1 = pd.DataFrame({'x': [True, False]})
df2 = pd.DataFrame({'x': [True, False, None]})
df1.merge(df2)
```

This is especially bad because pandas uses `None` as NULL-placeholder in bool-columns.

Commit message: REGR: allow merging on object boolean columns

Patch to review:
diff --git a/pandas/core/reshape/merge.py b/pandas/core/reshape/merge.py
index 4d8897fb7c811..cf07a8645b3af 100644
--- a/pandas/core/reshape/merge.py
+++ b/pandas/core/reshape/merge.py
@@ -28,6 +28,7 @@
     is_int_or_datetime_dtype,
     is_dtype_equal,
     is_bool,
+    is_bool_dtype,
     is_list_like,
     is_datetimelike,
     _ensure_int64,
@@ -974,9 +975,11 @@ def _maybe_coerce_merge_keys(self):
 
             # Check if we are trying to merge on obviously
             # incompatible dtypes GH 9780, GH 15800
-            elif is_numeric_dtype(lk) and not is_numeric_dtype(rk):
+            elif ((is_numeric_dtype(lk) and not is_bool_dtype(lk))
+                    and not is_numeric_dtype(rk)):
                 raise ValueError(msg)
-            elif not is_numeric_dtype(lk) and is_numeric_dtype(rk):
+            elif (not is_numeric_dtype(lk)
+                    and (is_numeric_dtype(rk) and not is_bool_dtype(rk))):
                 raise ValueError(msg)
             elif is_datetimelike(lk) and not is_datetimelike(rk):
                 raise ValueError(msg)
diff --git a/pandas/tests/reshape/merge/test_merge.py b/pandas/tests/reshape/merge/test_merge.py
index 8e639edd34b18..782d95b069b81 100644
--- a/pandas/tests/reshape/merge/test_merge.py
+++ b/pandas/tests/reshape/merge/test_merge.py
@@ -1526,6 +1526,17 @@ def test_merge_on_ints_floats_warning(self):
             result = B.merge(A, left_on='Y', right_on='X')
             assert_frame_equal(result, expected[['Y', 'X']])
 
+    def test_merge_incompat_infer_object(self):
+        # GH21119
+        df1 = DataFrame({'key': Series([True, False], dtype=object)})
+        df2 = DataFrame({'key': [True, False]})
+
+        expected = DataFrame({'key': [True, False]}, dtype=object)
+        result = pd.merge(df1, df2, on='key')
+        assert_frame_equal(result, expected)
+        result = pd.merge(df2, df1, on='key')
+        assert_frame_equal(result, expected)
+
     @pytest.mark.parametrize('df1_vals, df2_vals', [
         ([0, 1, 2], ["0", "1", "2"]),
         ([0.0, 1.0, 2.0], ["0", "1", "2"]),


Code review comments:
diff_hunk: @@ -1526,6 +1526,17 @@ def test_merge_on_ints_floats_warning(self):
             result = B.merge(A, left_on='Y', right_on='X')
             assert_frame_equal(result, expected[['Y', 'X']])
 
+    def test_merge_incompat_infer_object(self):
path: pandas/tests/reshape/merge/test_merge.py
text: @user1:
this is not a great test name, it should be related to bool.