Use concatenate_lookup in concatenate#6339
Conversation
|
Thanks @jakirkham. I can confirm that this fixes things for dask/dask-ml#685. |
|
Thanks for confirming Tom 🙂 Looks like it is passing on CI too. Travis CI isn't showing up for me on the status indicator, but the build did pass here. I'll work on some tests as well. |
concatenate_lookup in concatenateconcatenate_lookup in concatenate
|
@pentschev @cjnolet, it would be good if you could both take a look 🙂 |
588db14 to
52c1ef1
Compare
As we sometimes need to have custom ways to dispatch `concatenate` over different array types like SciPy's or CuPy's sparse matrices, make sure we lookup the appropriate `concatenate` implementation and use that.
52c1ef1 to
60fdbf1
Compare
|
Have added tests for SciPy and CuPy sparse matrices. Also have tested with a pre-release of CuPy 8. Since CuPy sparse matrices need PR ( cupy/cupy#2665 ) for concatenation support. |
60fdbf1 to
e3f3e36
Compare
pentschev
left a comment
There was a problem hiding this comment.
Changes make sense to me, thanks for doing that @jakirkham .
TomAugspurger
left a comment
There was a problem hiding this comment.
LGTM. One question inline.
| meta = np.concatenate([meta_from_array(s) for s in seq], axis=axis) | ||
| seq_metas = [meta_from_array(s) for s in seq] | ||
| _concatenate = concatenate_lookup.dispatch( | ||
| type(max(seq_metas, key=lambda x: getattr(x, "__array_priority__", 0))) |
There was a problem hiding this comment.
Do we do similar things with __array_priority__ in other places where we use concat?
There was a problem hiding this comment.
Yeah I actually took it from some concatenation code for creating a concrete result from a Dask Array above
Lines 298 to 300 in 4d7e674
|
This LGTM. Thanks @jakirkham! |
|
Thanks @jakirkham! |
|
Thanks all! 😄 |
* Use `concatenate_lookup` in `concatenate` As we sometimes need to have custom ways to dispatch `concatenate` over different array types like SciPy's or CuPy's sparse matrices, make sure we lookup the appropriate `concatenate` implementation and use that.
Fixes #5008
As we sometimes need to have custom ways to dispatch
concatenateover different array types like SciPy's or CuPy's sparse matrices, make sure we lookup the appropriateconcatenateimplementation and use that.black dask/flake8 dask