Support meta in apply gufunc#6521
Conversation
dask/array/tests/test_gufunc.py
Outdated
| assert mean.compute().shape == (10, 20) | ||
| assert std.compute().shape == (10, 20) | ||
| for expected, actual in zip(meta, result): | ||
| assert expected.dtype == actual._meta.dtype |
There was a problem hiding this comment.
Maybe we could do something like
expected = stats(a.compute())
assert_eq(expected[0], result[0])
assert_eq(expected[1], result[1])Using assert_eq checks lots of things, like shape, dtype, and other metadata. It's generally good testing hygiene in Dask to use it when possible.
There was a problem hiding this comment.
Thanks for the feedback! I switched to using assert_eq as suggested, which actually exposed the fact that passing np.float32 as the dtype for the second meta was not actually surviving after calling compute. I had to pass in the dtype in the gufunc and in the meta for it to actually take effect. The test illustrates that now, but let me know if you think it makes sense to add more tests to illustrate the usage.
|
This seems good to me. Thank you for adding this @joshreback . I'm good to merge as-is, although if you wanted to implement the suggestion around assert_eq I think that that would be marginally better. |
Hi @mrocklin - are there any other feedback or suggestions that I should address here? |
|
I think we're good, thanks for following up. |
|
Thanks!
…On Thu, Aug 20, 2020 at 1:28 PM Tom Augspurger ***@***.***> wrote:
Merged #6521 <#6521> into master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6521 (comment)>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACKZTFI3SZHC75V2NUULGTSBWBNRANCNFSM4QAZ7R5Q>
.
|
|
Very cool! Thanks Josh (and everyone who reviewed)! 😄 cc @pentschev (for vis) |
|
It's really awesome to see more |
5 participants
black dask/flake8 daskcloses #6358