-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix: fixed test_torch___getitem__
for paddle backend
#28631
fix: fixed test_torch___getitem__
for paddle backend
#28631
Conversation
5c9d3e0
to
328f202
Compare
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.
Hi @ZenithFlux
Thank you very much for the PR.
The changes here are critical and seem to have lead to many many failures for paddle tests that you can check under logs here at https://github.com/unifyai/ivy/actions/runs/8326992315/job/22798877642?pr=28631
Check Display Test Failures for a list of functions failing.
f685516
to
282e0d9
Compare
Fixed the syntax for checking if x is a True. Added a function to remove negative values in `query` of `paddle.get_item`.
282e0d9
to
bda8b7b
Compare
@Ishticode I don't think these failures are due to changes on this branch. I confirmed this by reverting all my changes and adding only a comment to both the functions (to trigger the same tests). Even without any changes to the code, there were still about a 100 tests failing. I also ran the tests for the some other failing functions on my local machine and I couldn't find any failure that was newly introduced by this branch. On the contrary, the tests I ran for If there's is still something I am missing I will be happy to fix that. |
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.
Right, otherwise looks good. I will merge it after quick few checks later today :)
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.
tests seems fine and i've checked with some custom examples. Looks good. I will merge now. Thank you very much for the effort put into this contribution @ZenithFlux :)
PR Description
query
ofpaddle.get_item
because paddle currently throws an error if any of the value in the arrays inside the query is negative.Related Issue
Closes #28630
Checklist
Socials