WIP: reduce unwanted type conversions of pd.Series.isin
State: closed

Addresses the bug in pd.Series.isin(values) that sometimes undesirably converts the dtype of `values` to the the dtype of the series before the `isin` comparison. Here is an example where `values` is converted to `int`, and hence the output `(True, True)` is yielded, while `(True, False)` is expected:
```python
>>>  values = [1, 0.5]
>>> pd.Series([1, 0]).isin(values)
0    True
1    True
dtype: bool
```

In my understanding, the bug has two sources:

**1) L407-408 in `pandas/core/algorithms.py`**

```python
comps, dtype, _ = _ensure_data(comps)
values, _, _ = _ensure_data(values, dtype=dtype)
```
Here, first the dtype of the series (`comps`) is determined, and then `values` is forcefully converted to that dtype. This e.g. leads to the undesired conversion of the example above.

**2) L417-434 in `pandas/core/algorithms.py`**
```python
    elif is_integer_dtype(comps):
        try:
            values = values.astype('int64', copy=False)
            comps = comps.astype('int64', copy=False)
            f = lambda x, y: htable.ismember_int64(x, y)
        except (TypeError, ValueError):
            values = values.astype(object)
            comps = comps.astype(object)

    elif is_float_dtype(comps):
        try:
            values = values.astype('float64', copy=False)
            comps = comps.astype('float64', copy=False)
            checknull = isna(values).any()
            f = lambda x, y: htable.ismember_float64(x, y, checknull)
        except (TypeError, ValueError):
            values = values.astype(object)
            comps = comps.astype(object)
```
Here, if the dtype of the series (`comps`) is either `float` or `int`, a forced conversion of `values` takes place again.

The fix:
**1) L408ff**
Only force the dtype of `comps` to `values` if *both* are int-like, float-like, or datetime-like.  If not, use `_ensure_data(values)`  *without* the `dtype` argument. I tried to keep the cost of the dtype checks low by exploiting lazy evaluation.

**2) L432ff** 
Add `and` statements such that the conversion only takes place when *both* `comps` and `values` are either `float` or `int`. If they are none of the above is true, both are converted to `object` type in the `else` block. This ensures that `return f(comps, values)` works (since `f = lambda x, y: htable.ismember_object(x, values)` from L426).

- [x] fixes #19356
- [x] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Bug
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
Hello @jdoepfert! Thanks for updating the PR.

 - In the file [`pandas/core/algorithms.py`](https://github.com/pandas-dev/pandas/blob/80d9c3d34102025dd082ab07cb38ba5dca3821c4/pandas/core/algorithms.py), following are the PEP8 issues :

> [Line 419:13](https://github.com/pandas-dev/pandas/blob/80d9c3d34102025dd082ab07cb38ba5dca3821c4/pandas/core/algorithms.py#L419): [E265](https://duckduckgo.com/?q=pep8%20E265) block comment should start with '# '





##### Comment last updated on February 05, 2018 at 01:05 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=h1) Report
> Merging [#19508](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f6b260bb8e048dc2efcedd76df5b878ba67fd43e?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/19508/graphs/tree.svg?height=150&width=650&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #19508      +/-   ##
==========================================
- Coverage   91.67%   91.65%   -0.02%     
==========================================
  Files         148      148              
  Lines       48553    48569      +16     
==========================================
+ Hits        44513    44518       +5     
- Misses       4040     4051      +11
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.02% <100%> (-0.02%)` | :arrow_down: |
| #single | `41.72% <100%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/algorithms.py](https://codecov.io/gh/pandas-dev/pandas/pull/19508/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWxnb3JpdGhtcy5weQ==) | `92.9% <100%> (-1.25%)` | :arrow_down: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/19508/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `82.23% <0%> (-0.18%)` | :arrow_down: |
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/19508/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `97% <0%> (-0.09%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/19508/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.25% <0%> (+0.01%)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/19508/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `95.74% <0%> (+0.21%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=footer). Last update [f6b260b...80d9c3d](https://codecov.io/gh/pandas-dev/pandas/pull/19508?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback Added a description. Agree, this definitely needs a performance check, will work on that tomorrow

EDIT: can't run asv at the moment, see #19518 
Still not sure I got it. Tried to implement the set check approach, but due to this test (L502 in `test_algos.py`)
```python
    def test_i8(self):

        arr = pd.date_range('20130101', periods=3).values
        result = algos.isin(arr, [arr[0]])
        expected = np.array([True, False, False])
```
I still had to to some datetime-like type checking:
Here, the dtype of `comps` is initially `datetime64[ns]`, but ensure_data makes it `int64` (`<M8[ns]`).  The dtype of the items in `values` is different, so no conversion is enforced. 
Hence I do not only check for exact equality of the dtypes (that ensure_data and the set check return) but I additionally use `is_time_like()`. 

Some tests are still failing, but if that's the way to go, I'll fix this up
Won't have time to work on this for the next 2 months, so if anyone wants to take over, give it a go! The last commit with all tests passing was https://github.com/pandas-dev/pandas/pull/19508/commits/cd14c56f5b3f8415e36b20934e8a011a462603d4.
closing as stale.