Skip to content
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

initial implementation of unique #171

Closed
wants to merge 3 commits into from
Closed

Conversation

Ferox98
Copy link

@Ferox98 Ferox98 commented Feb 15, 2022

Modified tests pass for numpy and jax but fail for pytorch and tensorflow. PyTorch's and Tensorflow's implementations of unique are somewhat different from numpy's and jax's. I used a few tricks to make them similar on PyTorch but get an error regarding the dtype of the torch tensor, which for some reason is generating a KeyError. Your help would be highly appreciated.

@Ferox98 Ferox98 changed the title first commit initial implementation of unique Feb 15, 2022
@djl11
Copy link
Contributor

djl11 commented Feb 22, 2022

Thanks a lot for the PR!

I'll need to rebase this to the latest master branch before continuing. Could you please give me write access to the PR? 🙂

@djl11
Copy link
Contributor

djl11 commented Feb 23, 2022

Thanks a lot for your contribution. I'll check it out in more detail soon. In the meantime, could you make sure the methods all follow the formatting explained in the contributor guide in the docs? I also go through some examples in the Array API tutorial series on YouTube.

Essentially:

  • All methods should be defined with def func_name explicitly, no lambdas or direct binding.
  • All Ivy methods should have type-hints and doc strings
  • All backend methods should have type-hints, but no doc strings.
  • All Array API methods should be moved to the correct sub-modules and files.

Please check the contributor guide in the docs for more details. Thanks!

Additionally, it may be necessary to resolve new conflicts with master since the PR was made. As a guide for resolving conflicts with upstream, I have created this short YouTube series.

Your PR may already be perfect, and if this is the case then no need to do anything!

Some tests will be failing, and thus the X shown, this is fine, as long as the tests for your own function are passing.

I'll take a look at the tests soon and see if it's ready to be merged :)

Copy link
Contributor

@djl11 djl11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all changes to .idea.

Please also reformat the code as describe in the contributor guide in the docs, and also as explained in this YouTube series.

@djl11 djl11 added Array API Conform to the Array API Standard, created by The Consortium for Python Data API Standards Single Function labels Feb 24, 2022
@1Doomdie1
Copy link
Contributor

Closing PR due to inactivity.

@1Doomdie1 1Doomdie1 closed this Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Array API Conform to the Array API Standard, created by The Consortium for Python Data API Standards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants