-
Notifications
You must be signed in to change notification settings - Fork 11
ENH: add new function one_hot
#306
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
base: main
Are you sure you want to change the base?
Conversation
9b5f393
to
02544de
Compare
RE dtype, I think data-apis/array-api#848 will give us something slightly cleaner down the line. In SciPy we have been using https://github.com/scipy/scipy/blob/main/scipy/_lib/_array_api.py#L399 with |
Yeah, I'm not 100% sure how those links will help in this case? We're not casting one thing to another kind, or promoting to float. We just want the default float dtype irrespective of what was passed in. You may want to consider giving names to:
I'm not sure though, and these are one-liners. |
the point is that instead of writing dtype = xp.empty(()).dtype
x = xp.zeros(..., dtype=dtype) we would have x = xp.zeros(...)
x = xp.astype(x, 'real floating') |
Ah, right. Okay. I guess you mean Also, could you help me solve the Dask errors? This is all foreign to me. And how do I make an array with a non-concrete size? ( |
Nope, data-apis/array-api#848 suggests overloading the |
Oh, no worries, I don't have a strong opinion. Just trying to keep up with all the planned changes 😄 Did you see my edits? I could use some guidance with the Dask errors. |
to me this doesn't make much sense. Why shouldn't it be |
I'd rather follow what the libraries are doing than force double conversion for delegated code. If you do that, most people would end up having to write their own In general, the reason it's not bool is because these values often serve as the inputs to machine learning algorithms. |
@lucascolley Ready for your review |
if supports_fancy_indexing: | ||
out = at(out)[xp.arange(x_size), x_flattened].set(1) | ||
else: | ||
for i in range(x_size): | ||
x_i = x_flattened[i] | ||
if not supports_array_indexing: | ||
x_i = int(x_i) | ||
out = at(out)[i, x_i].set(1) |
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.
@crusaderky have we encountered patterns like this before?
@pytest.mark.skip_xp_backend( | ||
Backend.DASK, reason="backend does not yet support indexed assignment" | ||
) |
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 this consistent with our skips elsewhere for the same reason @crusaderky ? (I'm on mobile, not super easy to check)
Fixes #305
Questions:
xp.empty(()).dtype
?