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.

Sign up for GitHub

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

Jump to bottom

feat: add paddle tolist #26606

Merged
merged 2 commits into from
Oct 6, 2023
Merged

feat: add paddle tolist #26606

merged 2 commits into from
Oct 6, 2023

Conversation

Anteemony
Copy link
Contributor

@Anteemony Anteemony commented Oct 4, 2023 •
edited
Loading

PR Description

Related Issue

Close #23649

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you run your tests and are your tests passing?
  • Did pre-commit not fail on any check?
  • Did you follow the steps we provided?

Socials

github-actions[bot]
github-actions bot reviewed Oct 4, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

@Anteemony Anteemony changed the title Add paddle tolist feat: add paddle tolist Oct 4, 2023
@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Oct 4, 2023
@Anteemony
Copy link
Contributor Author

Anteemony commented Oct 4, 2023 •
edited
Loading

Hello, I believe the errors come from helpers.test_frontend_function during the tests.
Paddle.tolist returns a python list or number and not an array. So the test's array operations will not work properly.
The test passes when I add test_values=False to helpers.test_frontend_function in the test file.
Checking the frontend function manually also gives expected results.
Should I go ahead with this solution?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023 •
edited
Loading

Thank you for this PR, here is the CI results:

This pull request does not result in any additional test failures. Congratulations!

@Anteemony
Copy link
Contributor Author

Hello @Daniel4078 please could you leave a review. I don’t know why

@Anteemony Anteemony closed this Oct 5, 2023
@Anteemony
Copy link
Contributor Author

Anteemony commented Oct 5, 2023 •
edited
Loading

Hi @Daniel4078 could you please guide me on what’s happening? I don’t know why it’s causing failed tests

@Anteemony Anteemony reopened this Oct 5, 2023
@Daniel4078
Copy link
Contributor

Sorry for the delay, I just wake up. Will check it out now.

@Daniel4078
Copy link
Contributor

I did not see any tolist tests failing on my end though? Can you give some more context about the problem you faced?

@Anteemony
Copy link
Contributor Author

I used to get an Assertion error before with the info that the data types and array sizes do not match.
But I reran the tests today and I'm getting a different error on my codespace.

@Anteemony
Copy link
Contributor Author

image

I wonder if this has anything to do with my setup. And sorry for the late reply on my end too @Daniel4078

@Anteemony
Copy link
Contributor Author

Hello @Daniel4078 could you please leave a review when you can?

Daniel4078
Daniel4078 approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@Daniel4078 Daniel4078 left a comment

Choose a reason for hiding this comment

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

Looks good to me now with no relavent tests failing in the checks

@Daniel4078 Daniel4078 merged commit 94c5a34 into ivy-llc:main Oct 6, 2023
48 of 50 checks passed
druvdub pushed a commit to druvdub/ivy that referenced this pull request Oct 14, 2023
@Anteemony @druvdub
feat: add paddle tolist (ivy-llc#26606)
1075eac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Daniel4078 Daniel4078 Daniel4078 approved these changes

Assignees

@Daniel4078 Daniel4078

Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tolist
3 participants
@Anteemony @Daniel4078 @ivy-leaves