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

DOC: Improve docs for jnp.trim_zeros #23540

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

rajasekharporeddy
Copy link
Contributor

@rajasekharporeddy rajasekharporeddy commented Sep 10, 2024

Copy link
Collaborator

@jakevdp jakevdp left a comment

Choose a reason for hiding this comment

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

Thanks!

@google-ml-butler google-ml-butler bot added kokoro:force-run pull ready Ready for copybara import and testing labels Sep 10, 2024
@jakevdp
Copy link
Collaborator

jakevdp commented Sep 10, 2024

Looks like the new type annotations led to some mypy failures. I'd remove the type annotations from this doc PR, and add them back in another PR if you're interested in doing that work.

@rajasekharporeddy
Copy link
Contributor Author

rajasekharporeddy commented Sep 10, 2024

Thanks! I would remove the annotation in this PR and open another PR for that. Also, jnp.tim_zeros works for python list inputs also.

>>> jnp.trim_zeros([0, 2, 3, 1, 0, 0])
Array([2, 3, 1], dtype=int32)

Since JAX APIs doesn't take python lists as input, can we make this compatible with other APIs by introducing an array check using check_arraylike for this API?

@jakevdp
Copy link
Collaborator

jakevdp commented Sep 10, 2024

Yeah, good catch! We should definitely restrict this to arraylike inputs, but that will require a deprecation cycle (you can see similar things in process for other functions in this file)

@rajasekharporeddy
Copy link
Contributor Author

Thanks @jakevdp. I will do that.

Copy link
Collaborator

@jakevdp jakevdp left a comment

Choose a reason for hiding this comment

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

Thanks! I'll look for your PRs with the followup fixes to annotations and checking arraylike inputs.

@copybara-service copybara-service bot merged commit a8b68c2 into google:main Sep 10, 2024
12 of 14 checks passed
@rajasekharporeddy rajasekharporeddy deleted the testbranch2 branch September 10, 2024 17:16
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants