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

added function paddle.vision.transforms.crop and test draft #21821

Closed
wants to merge 36 commits into from
Closed

added function paddle.vision.transforms.crop and test draft #21821

wants to merge 36 commits into from

Conversation

danieldaug
Copy link

Close #21818

@github-actions
Copy link
Contributor

Thanks for contributing to Ivy! 😊👏
Here are some of the important points from our Contributing Guidelines 📝:
1. Feel free to ignore the run_tests (1), run_tests (2), … jobs, and only look at the display_test_results job. 👀 It contains the following two sections:
- Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
- New Failures Introduced: This lists the tests that are passing on master, but fail on the PR Fork. Please try to make sure that there are no such tests. 💪
2. The lint / Check formatting / check-formatting tests check for the formatting of your code. 📜 If it fails, please check the exact error message in the logs and fix the same. ⚠️🔧
3. Finally, the test-docstrings / run-docstring-tests check for the changes made in docstrings of the functions. This may be skipped, as well. 📚
Happy coding! 🎉👨‍💻

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Aug 13, 2023
@danieldaug danieldaug marked this pull request as draft August 15, 2023 02:52
@danieldaug danieldaug requested a review from AnnaTz August 15, 2023 02:55
@AnnaTz AnnaTz marked this pull request as ready for review August 15, 2023 09:22
@danieldaug danieldaug requested a review from AnnaTz August 17, 2023 00:32
@AnnaTz
Copy link
Contributor

AnnaTz commented Aug 17, 2023

Hi @danieldaug, please resolve the conflicts so that the CI checks can properly run.

@danieldaug danieldaug requested a review from AnnaTz August 19, 2023 00:26
AnnaTz
AnnaTz reviewed Aug 23, 2023
View reviewed changes
ivy/data_classes/container/experimental/utility.py
ret
Output container.
"""
return self.static_optional_get_element(self, out=out)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this was here before, but what are these optional_get changes? It's not related to crop is it?

Copy link
Author

Choose a reason for hiding this comment

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

I assumed that these changes were due to me pulling other changes from the main branch over time since I did not manually make any of these changes. Will these be a problem as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Your main branch is not synced with ours.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

and you also seem to have pushed changes to the test_array_api files. We only merge PRs that make changes related to the task they are resolving. Anything extra needs to be removed.

Copy link
Author

Choose a reason for hiding this comment

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

I have synced my main branch although I am unable to access and reset or revert test_array_api as it is a subfolder containing 31 files that I have not touched. Could this problem be due to other users participating in my pull-request? Sorry for all the trouble, but all of these problems besides the lint errors have not been due to changes made by me so I am very confused as to why they are happening.

Copy link
Contributor

@AnnaTz AnnaTz Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I would suggest making a new branch for each task you start working on. This will make it easier to resolve such issues in the future.
I think the most straightforward way to deal with this now is to 1) make a new fork, 2) create a branch and copy-paste your function and test there, 3) make a new PR from this branch.

@ivy-seed ivy-seed added the Stale label Oct 3, 2023
@ivy-seed
Copy link

ivy-seed commented Oct 3, 2023

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.

@danieldaug danieldaug closed this by deleting the head repository Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnnaTz AnnaTz AnnaTz requested changes

@Aarsh2001 Aarsh2001 Awaiting requested review from Aarsh2001

@zaeemansari70 zaeemansari70 Awaiting requested review from zaeemansari70

@fnhirwa fnhirwa Awaiting requested review from fnhirwa

Assignees

@AnnaTz AnnaTz

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

Successfully merging this pull request may close these issues.

paddle.vision.transforms.crop
6 participants
@danieldaug @AnnaTz @ivy-seed @ivy-leaves @root-yash @KareemMAX