BUG: Use Series.where rather than np.where in clip
State: closed

Closes #3996


Labels:

Comments:
can u add release notes mention

and another test that has NaT in the series?

Good catch on NaT. I'll add some tests. Just to confirm, what behavior do you want in that case? This commit is currently consistent with 11.0 behavior, which is itself consistent with the fact that NaT is ordered strictly lower than other timestamps.

```
>>> pd.__version__
'0.11.0'

>>> s
0                   NaT
1   2000-01-02 00:00:00
2   2000-01-03 00:00:00
dtype: datetime64[ns]
>>> s.clip_lower(s[1].value)
0   2000-01-02 00:00:00
1   2000-01-02 00:00:00
2   2000-01-03 00:00:00
dtype: datetime64[ns]
>>> s.clip_upper(s[1].value)
0                   NaT
1   2000-01-02 00:00:00
2   2000-01-02 00:00:00
dtype: datetime64[ns]
>>> s.clip_lower(pd.NaT.value)
0                   NaT
1   2000-01-02 00:00:00
2   2000-01-03 00:00:00
dtype: datetime64[ns]
>>> s.clip_upper(pd.NaT.value)
0   NaT
1   NaT
2   NaT
dtype: datetime64[ns]
```

At the expense of explicit null checking and some backwards incompatibility, we could instead make the behavior more nan-like. Thoughts?

these should work with out u specifying value directly
eg s[1]

as far as NaT go they should be completely excluded 
so this might be a bit more complicated

u need to compute the mask first, then do the where using that mask (so that those values not in the mask are only ones included), then put the Nan's back in place

this should work for datetimes and other dtypes

I'll give u a reference to look at

actually I think this easier that I said

just make the cond = (s > 0) & notnull(s)

I think will do the right thing

I have it coded like this:

```

    def clip_upper(self, threshold):
        cond = (self < threshold) | isnull(self) | isnull(threshold)
        return self.where(cond, threshold)

    def clip_lower(self, threshold):
        cond = (self > threshold) | isnull(self) | isnull(threshold)
        return self.where(cond, threshold)
```

I'll finish up the PR tomorrow.

ok that looks right

throw in some test cases including some with nan (and different dtypes) I u can
thanks!

see if you can finish this up...want to get it itn

Build failed in Python 3 for the case when the threshold is None. What behavior do you want in that case? Throw an exception? Return a copy of the series? Or just take it out of the test as a nonsensical condition?

if the threshold is `None` (which btw ONLY exists in an `object` series, otherwise its converted to `nan`), or `nan`, then I think the operation is undefined. Can't really compare to `nan`; so either treat it as `inf`, which means do nothing or raise. It think raising a `ValueError` would be fine.

Python 3 fails test, ~~I think because of `(self >= threshold)`, but then this worked before hmm~~:

```
======================================================================
ERROR: test_clip_types_and_nulls (pandas.tests.test_series.TestSeries)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.2_with_system_site_packages/lib/python3.2/site-packages/pandas-0.11.1.dev_f4ed17b-py3.2-linux-x86_64.egg/pandas/tests/test_series.py", line 2851, in test_clip_types_and_nulls
    l = s.clip_lower(thresh)
  File "/home/travis/virtualenv/python3.2_with_system_site_packages/lib/python3.2/site-packages/pandas-0.11.1.dev_f4ed17b-py3.2-linux-x86_64.egg/pandas/core/series.py", line 2002, in clip_lower
    cond = (self >= threshold) | isnull(self) | isnull(threshold)
  File "/home/travis/virtualenv/python3.2_with_system_site_packages/lib/python3.2/site-packages/pandas-0.11.1.dev_f4ed17b-py3.2-linux-x86_64.egg/pandas/core/series.py", line 236, in wrapper
    res = na_op(values, other)
  File "/home/travis/virtualenv/python3.2_with_system_site_packages/lib/python3.2/site-packages/pandas-0.11.1.dev_f4ed17b-py3.2-linux-x86_64.egg/pandas/core/series.py", line 206, in na_op
    result = lib.scalar_compare(x, y, op)
  File "lib.pyx", line 610, in pandas.lib.scalar_compare (pandas/lib.c:10608)
TypeError: unorderable types: str() >= NoneType()
----------------------------------------------------------------------
```

Yep. I don't have a Python 3 environment yet. Building one now...

Is that really true, @jreback? It short-circuits at the element level? 

fyi you also might be interestd in a `vagrantup.com` environment....pretty easy to setup

yes

```
In [6]: s = Series([None,'a','b','c'])

In [7]: pd.notnull(s) & (s>'aaa')
Out[7]: 
0    False
1    False
2     True
3     True
dtype: bool

In [8]: s>'aaa'
Out[8]: 
0    False
1    False
2     True
3     True
dtype: bool
```

@mtki you resolved the python3 issue?

Builds are now passing.

@mtkni needs a rebase (probably due to conflict in release.rst)

Rebased.

Great!