-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 np.asfarray impl #5418
Add np.asfarray impl #5418
Conversation
Ready for a round of reviews |
Thanks for the patch.
What does this mean?! I can't see related changes to the fundamental type in the patchset. |
I've undone those changes because it was breaking some tests. |
PR is ready for a review |
Thanks for the review @luk-f-a. I've fixed the code to support complex types. |
@guilhermeleobas thanks for the changes, it looks good to me. |
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.
LGTM. Thanks!
Hi, @stuartarchibald When you have some cycles to spare, can you take a look in this PR? |
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 implementing this, I've given it an initial review, looks good, just one thing to resolve.
numba/np/arraymath.py
Outdated
@@ -3899,6 +3899,19 @@ def impl(a, dtype=None): | |||
return impl | |||
|
|||
|
|||
@overload(np.asfarray) | |||
def np_asfarray(a, dtype=types.float64): |
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.
think this ought to be dtype=np.float64
?
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.
and then something check that the thing passed in is actually a dtype or can be accepted and converted to one?
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 the check necessary? if dtype
is not a subdtype
of np.inexact
, then, dtype
will be assigned to types.float64
.
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.
ok, I think the check can be skipped as the as_dtype
will pick up nonsense types.
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 the fixes, looks good!
Note: CI fail is #5973 |
Thanks for the review @stuartarchibald |
As title. I've updated
types.float_
to betypes.float64
instead oftypes.float32
. On NumPy, at least on my machine,np.float_
isnp.float64
.