Min max sparse #41159
Min max sparse #41159
Conversation
taytzehao
commented
Apr 26, 2021
taytzehao
commented
- closes BUG: max()/min() don't work on DataFrames constructed from scipy sparse matrix #40921
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
|
Hello @taytzehao! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-04-27 08:13:18 UTC |
doc/source/whatsnew/v1.3.0.rst
Outdated
| @@ -874,7 +874,7 @@ Sparse | |||
|
|
|||
| - Bug in :meth:`DataFrame.sparse.to_coo` raising ``KeyError`` with columns that are a numeric :class:`Index` without a 0 (:issue:`18414`) | |||
| - Bug in :meth:`SparseArray.astype` with ``copy=False`` producing incorrect results when going from integer dtype to floating dtype (:issue:`34456`) | |||
| - | |||
| - Bug in :meth:`max` and `min` for SparseArrat type do not exist (:issue:`40921`) | |||
There was a problem hiding this comment.
spelling here. also pls use the direct reference e.g. :class:`SparseArray`
|
|
||
|
|
||
| def test_maxmin(): | ||
| data = np.arange(10).astype(float) |
There was a problem hiding this comment.
can you make sure some NaNs are in this, and assert that we are correctly hitting those (can parameterize to do that)
jreback
added
Sparse
|
pre-commit is failing, pls run the hooks: https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#pre-commit |
doc/source/whatsnew/v1.3.0.rst
Outdated
| @@ -877,7 +877,7 @@ Sparse | |||
|
|
|||
| - Bug in :meth:`DataFrame.sparse.to_coo` raising ``KeyError`` with columns that are a numeric :class:`Index` without a 0 (:issue:`18414`) | |||
| - Bug in :meth:`SparseArray.astype` with ``copy=False`` producing incorrect results when going from integer dtype to floating dtype (:issue:`34456`) | |||
| - | |||
| - Bug in :class:`SparseArray` type as :meth:`max` and :meth:`min` do not exist (:issue:`40921`) | |||
There was a problem hiding this comment.
suggestion: "Implemented :meth:`SparseArray.min` and :meth:`SparseArray.max` (:issue:`40921`)"
pandas/core/arrays/sparse/array.py
Outdated
| if self.sp_index.ngaps > 0 and np.all(self._valid_sp_values < 0): | ||
|
|
||
| if self.size > 0 and self._valid_sp_values.size == 0: | ||
| return np.nan |
There was a problem hiding this comment.
wouldnt self.fill_value be relevant here?
pandas/core/arrays/sparse/array.py
Outdated
|
|
||
| return 0 | ||
| else: | ||
| return np.amax(self._valid_sp_values, axis) |
There was a problem hiding this comment.
any particular reason for np.amax instead of np.max?
|
thanks @taytzehao |
|
@jreback this wasnt ready, in particular #41159 (comment) looks sketchy |
|
ok feel free to revert |
|
@taytzehao would you prefer to revert or to address this in a follow-up? |
