-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
BUG: Add SparseArray.all #17570
BUG: Add SparseArray.all #17570
Conversation
pandas/core/sparse/array.py
Outdated
values = self.sp_values | ||
|
||
if len(values) != len(self): | ||
values = values.tolist() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason you are converting to a list? and not a dense array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback For the performance reason. This is sparse and self.to_dense()
is not efficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am asking why you are converting .tolist()
, then appending, that is non-performant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply I wanted to use np.all
. I got an idea not to use tolist()
. I'll change the solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls add a whatsnew note. do we have a specific issue for this? (other that the PR)?
@@ -614,6 +614,24 @@ def fillna(self, value, downcast=None): | |||
return self._simple_new(new_values, self.sp_index, | |||
fill_value=fill_value) | |||
|
|||
def all(self, axis=0, *args, **kwargs): | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might as well add any as well.
@jreback Thanks for your review.
|
b5bb960
to
19e3e44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comment. ping on green
doc/source/whatsnew/v0.21.0.txt
Outdated
@@ -544,6 +544,7 @@ Sparse | |||
|
|||
- Bug in ``SparseSeries`` raises ``AttributeError`` when a dictionary is passed in as data (:issue:`16905`) | |||
- Bug in :func:`SparseDataFrame.fillna` not filling all NaNs when frame was instantiated from SciPy sparse matrix (:issue:`16112`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
say that these are now implemented to handle SparseArray
|
||
Returns | ||
------- | ||
all : bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a See Also to point to np.all
Codecov Report
@@ Coverage Diff @@
## master #17570 +/- ##
==========================================
- Coverage 91.22% 91.2% -0.02%
==========================================
Files 163 163
Lines 49625 49642 +17
==========================================
+ Hits 45270 45277 +7
- Misses 4355 4365 +10
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17570 +/- ##
==========================================
- Coverage 91.24% 91.2% -0.05%
==========================================
Files 163 163
Lines 49766 49642 -124
==========================================
- Hits 45411 45276 -135
- Misses 4355 4366 +11
Continue to review full report at Codecov.
|
@jreback Thank you. These are now fixed. |
@jreback UPDATE: I found |
lgtm. waiting for CI to go green again before merging (unrelated conda-ish issues) |
can you rebase |
2babd92
to
94973e8
Compare
@jreback Rebased. |
thanks @Licht-T |
git diff upstream/master -u -- "*.py" | flake8 --diff
This is the part of #17386.
Block.where
usesndarray.all
, but there is no such method inSparseArray
.This makes that
all
evaluatesSparseArray([False, False, True])
asTrue
.https://github.com/pandas-dev/pandas/blob/master/pandas/core/internals.py#L1400