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

feat(paddle frontend): add index_sample function #26756

Closed
wants to merge 7 commits into from

Conversation

ksbharaj
Copy link

@ksbharaj ksbharaj commented Oct 7, 2023

PR Description

Added Index_sample for Paddle

Related Issue

Close #26495

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

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

@ksbharaj ksbharaj changed the title Issue #26495 (index_sample for Paddle) feat(paddle frontend): add index_sample function Oct 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2023

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


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

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Oct 9, 2023
@ksbharaj
Copy link
Author

Hi @Sam-Armstrong and @KareemMAX. I raised this PR to address the requested change. Kindly let me know if you require any additional info to help merge this to the main branch. Thank you

@Sam-Armstrong
Copy link
Contributor

@ksbharaj please could you add the corresponding frontend test?
https://unify.ai/docs/ivy/overview/deep_dive/ivy_frontends_tests.html#ivy-frontend-tests

@ksbharaj
Copy link
Author

Hi @Sam-Armstrong. I believe this is already there from a previous PR. Please check:

ivy/ivy_tests/test_ivy/test_frontends/test_paddle
/test_search.py

and the function test_paddle_index_sample()

Let me know if that's the correct test, and if anything else is needed.

@ksbharaj
Copy link
Author

Hi @Sam-Armstrong. I believe this is already there from a previous PR. Please check:

ivy/ivy_tests/test_ivy/test_frontends/test_paddle /test_search.py

and the function test_paddle_index_sample()

Let me know if that's the correct test, and if anything else is needed.

Hi @Sam-Armstrong. Have you had a chance to review this? Kindly let me know if you need any more information from me

@Sam-Armstrong
Copy link
Contributor

@ksbharaj we also need a test in ivy/ivy_tests/test_ivy/test_frontends/test_paddle
/test_tensor.py, as you've implemented a tensor method here.

@ksbharaj ksbharaj marked this pull request as draft October 14, 2023 16:35
@ksbharaj
Copy link
Author

Hey @Sam-Armstrong
I've added the test to test_paddle/test_tensor/test_tensor.py as requested. Kindly let me know if there's anything else required.

@ksbharaj ksbharaj marked this pull request as ready for review October 24, 2023 13:37
@ksbharaj
Copy link
Author

Hi @Sam-Armstrong. I added the tests, but I think this still needs some rectifications. I think I am facing some issues getting the tests to work locally, so I can't debug effectively.
If you have the chance, please review my code and kindly let me know how I can improve it (I'm eager to learn and improve here)

@ksbharaj
Copy link
Author

Hi @Sam-Armstrong. Please do let me know. I would like to continue contributing but just need some help on how to do it correctly. Thank you

@Sam-Armstrong
Copy link
Contributor

Hi @ksbharaj, sorry for the slow reply. I'm not exactly sure what's going wrong with the test unfortunately, the other tests in the file seem to be failing for me also. I'll continue looking into it, but maybe for the time being could you resolve the merge conflicts with main? Thanks!

@ivy-seed
Copy link

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

@Ishticode
Copy link
Contributor

Closing this due to inactivity for over 6 months. Please feel free to reopen if you would like to continue working on this. Thank you :)

@Ishticode Ishticode closed this Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

index_sample
7 participants