-
Notifications
You must be signed in to change notification settings - Fork 4
take()
implementation
#96
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
Conversation
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.
# [[1, 1], [0, 1]], [[1, 0], [0, 0]] | ||
# >>> torch.unique(torch.as_tensor(x), dim=2) | ||
# [[1, 1], [1, 0]], [[0, 1], [0, 0]] | ||
# |
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 something we need to fix in our unique?
The Notes section in https://numpy.org/doc/stable/reference/generated/numpy.unique.html is probably relevant, if somewhat obscure.
WDYT @lezcano ?
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.
(unique
against axis gave me a headache so I had left this xfail'd and documented incase you/Mario had an immediate answer 😅)
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.
Uf. This looks like a bit of a pain to implement. I'd say we don't touch it and leave it xfailed.
Rebased and merged in gh-98 |
Rudimentary
take()
implementations. Passes my Array APItest_take
at least (data-apis/array-api-tests#173, no means comprehensive for NumPy-propers semantics oftake()
), and un-xfailing existing tests didn't uncover anything—I imagine however importing more tests we'll find a bug or two for sure though.It's kinda awkward to implement
ndarray.take()
given the redundancies of handling the kwargs in_wrapper.take()
too (which cannot be used due to circular import), so left it not implemented...Hopefully this is how I should be implementing things now? I really like
@normalizer
!