Add compress
#3103
Add compress
#3103
Conversation
|
This PR is for the feature request Issue #2224 |
| if not isinstance(condition, ndarray): | ||
| condition = core.array(condition, dtype=int) | ||
| if condition.ndim != 1: | ||
| raise ValueError('condition must be a 1-d array') |
There was a problem hiding this comment.
We do not accept numpy arrays as argument in general.
I think we can just ensure isinstance(condition, ndarray) and raise TypeError.
There was a problem hiding this comment.
I am not sure if I understand what you said. Doesn't ndarray mean only cupy ndarrays.
There was a problem hiding this comment.
Yes, that's correct. ndarray is cupy.ndarray in this context.
There was a problem hiding this comment.
If we just assert that instance is of ndarray, then we might not be able to take a python list of booleans as input. That's why I thought I might convert it to cupy ndarray (which also has a side-effect of converting numpy arrays also to cupy ndarrays). Otherwise, I would need to check manually for a list of booleans.
There was a problem hiding this comment.
We do not accept list of booleans as well.
There was a problem hiding this comment.
Sorry, as take accepts both lists and NumPy arrays as indices, I think condition can support them as well.
|
Please test some additional configurations:
|
|
Thanks. Jenkins, test this please |
|
Successfully created a job for commit c4b826b: |
niboshi
removed
the
st:test-and-merge
|
Forgot one thing, could you add a documentation entry to |
|
Jenkins CI test (for commit c4b826b, target branch master) succeeded! |
Harshan01
force-pushed
the
cupy-compress
branch
from
498e465
to
9046147
Compare
|
@niboshi I have made the requested changes. Can you please review them? |
|
Thanks. Jenkins, test this please |
|
Successfully created a job for commit 9046147: |
|
Jenkins CI test (for commit 9046147, target branch master) succeeded! |
|
👍 |
I tried to mimic numpy as much as possible. I observed that
PyArray_Compressin numpy internally callsnonzeroandtake. So, I tried to do it in the same way instead of making a separate kernel. I would be happy to receive suggestions on how to implement the kernel forcompress, if it is required to do so.