-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add dtype and order to asarray and asanyarray definitions #8106
Conversation
Thanks Julia! 😄 This makes a lot of sense 🙂 |
@jrbourbeau do you have thoughts on whether we should include this in the upcoming release ( dask/community#180 )? |
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.
Thanks for fixing @jsignell!
I realize it predates this PR, but should we be forwarding like
, dtype
, etc. keywords for the case when the input a
is from xarray
?
return from_array(a, getitem=getter_inline, **kwargs) | ||
|
||
|
||
def asanyarray(a, *, like=None): | ||
def asanyarray(a, dtype=None, order=None, *, like=None): |
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.
Similar comment here re: docstring
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.
yeah totally! I might just steal the docstring from numpy though via a @derived_from
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.
In general I'm in favor of using derived_from
, though in this case we have an extra allow_unknown_chunksizes
keyword and it looks like like
has some Dask-specific verbiage. Maybe manually copying over the descriptions from NumPy makes more sense here?
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.
Actually I just copied them over since the like
is already customized.
Good point. We should be forwarding all the kwargs IMO. |
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.
This looks great, thanks everyone for tracking this down and @jsignell for fixing this! 😄
I requested a small change to test like=
with dtype=
, otherwise we're not actually testing the fix in here.
cf21af9
to
532d28c
Compare
Thanks Peter! 🙂 |
All credit goes to @jsignell , thanks Julia! 😄 |
TypeError
inasanyarray
__array_function__
#8105black dask
/flake8 dask
/isort dask
I'll leave this here for when @pentschev gets back.