Skip to content
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

DEPR: Series.compress #21930

Merged
merged 3 commits into from
Jul 25, 2018
Merged

DEPR: Series.compress #21930

merged 3 commits into from
Jul 25, 2018

Conversation

TomAugspurger
Copy link
Contributor

xref #18262

@TomAugspurger TomAugspurger added the Deprecate Functionality to remove in pandas label Jul 16, 2018
@TomAugspurger TomAugspurger added this to the 0.24.0 milestone Jul 16, 2018
@jreback jreback mentioned this pull request Jul 16, 2018
34 tasks
@codecov
Copy link

codecov bot commented Jul 16, 2018

Codecov Report

Merging #21930 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #21930      +/-   ##
==========================================
+ Coverage      92%      92%   +<.01%     
==========================================
  Files         170      170              
  Lines       50609    50611       +2     
==========================================
+ Hits        46561    46563       +2     
  Misses       4048     4048
Flag Coverage Δ
#multiple 90.4% <100%> (ø) ⬆️
#single 42.19% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/series.py 94.11% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e48393...c228d49. Read the comment docs.

@@ -314,7 +314,7 @@ Deprecations
- :meth:`DataFrame.to_stata`, :meth:`read_stata`, :class:`StataReader` and :class:`StataWriter` have deprecated the ``encoding`` argument. The encoding of a Stata dta file is determined by the file type and cannot be changed (:issue:`21244`).
- :meth:`MultiIndex.to_hierarchical` is deprecated and will be removed in a future version (:issue:`21613`)
- :meth:`Series.ptp` is deprecated. Use ``numpy.ptp`` instead (:issue:`21614`)
-
- :meth:`Series.compress` is deprecated. Use ``Series.__getitem__`` instead (:issue:`18262`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer what you said in the warning message, which is to use Series[condition] instead (more end-user friendly IMO vs. Series.__getitem__).

@jreback
Copy link
Contributor

jreback commented Jul 17, 2018

comment by @gfyoung but otherwise, merge away.

@jreback jreback merged commit 693e235 into pandas-dev:master Jul 25, 2018
@jreback
Copy link
Contributor

jreback commented Jul 25, 2018

thanks @TomAugspurger

@jorisvandenbossche
Copy link
Member

One problem with this PR is that now np.compress also raises a warning:

In [9]: cond = [True, False, True, False, False]
   ...:   s = Series([1, -1, 5, 8, 7], index=list('abcde'))

In [10]: np.compress(cond, s)
/home/joris/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py:57: FutureWarning: Series.compress(condition) is deprecated. Use Series[condition] instead.
  return getattr(obj, method)(*args, **kwds)
Out[10]: 
a    1
c    5
Name: foo, dtype: int64

so apparently np.compress will simply call the method on the passed object if that exists.

Is there something to do about that? (I suppose not?)

@jorisvandenbossche
Copy link
Member

Additional problem is that once we remove the method, np.compress will also not work anymore.

You get something like (with the method commented out):

In [2]: np.compress(cond, s)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
~/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py in _wrapfunc(obj, method, *args, **kwds)
     56     try:
---> 57         return getattr(obj, method)(*args, **kwds)
     58 

~/scipy/pandas/pandas/core/generic.py in __getattr__(self, name)
   4557                 return self[name]
-> 4558             return object.__getattribute__(self, name)
   4559 

AttributeError: 'Series' object has no attribute 'compress'

During handling of the above exception, another exception occurred:

ValueError                                Traceback (most recent call last)
<ipython-input-2-cb6f12fdb9e8> in <module>()
----> 1 np.compress(cond, s)

~/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py in compress(condition, a, axis, out)
   1668 
   1669     """
-> 1670     return _wrapfunc(a, 'compress', condition, axis=axis, out=out)
   1671 
   1672 

~/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py in _wrapfunc(obj, method, *args, **kwds)
     65     # a downstream library like 'pandas'.
     66     except (AttributeError, TypeError):
---> 67         return _wrapit(obj, method, *args, **kwds)
     68 
     69 

~/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py in _wrapit(obj, method, *args, **kwds)
     49         if not isinstance(result, mu.ndarray):
     50             result = asarray(result)
---> 51         result = wrap(result)
     52     return result
     53 

~/scipy/pandas/pandas/core/series.py in __array_wrap__(self, result, context)
    655         """
    656         return self._constructor(result, index=self.index,
--> 657                                  copy=False).__finalize__(self)
    658 
    659     def __array_prepare__(self, result, context=None):

~/scipy/pandas/pandas/core/series.py in __init__(self, data, index, dtype, name, copy, fastpath)
    264                             'Length of passed values is {val}, '
    265                             'index implies {ind}'
--> 266                             .format(val=len(data), ind=len(index)))
    267                 except TypeError:
    268                     pass

ValueError: Length of passed values is 2, index implies 5

which is not a very user friendly error message ..

Of course, if we are fine with np.compress(series) not working anymore, then the fact that it raises a deprecation message (my previous comment) is of course fine ..

@TomAugspurger
Copy link
Contributor Author

How about we update the warning message to be

Use `series[condition] or np.asarray(series).compress(condition)`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants