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

Adjusted the Image Contrast #22253

Closed
wants to merge 17 commits into from
Closed

Conversation

ahmad-zayab
Copy link

@ahmad-zayab ahmad-zayab commented Aug 20, 2023

Close #22205

@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 main, 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 20, 2023
@ivy-leaves
Copy link

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you 🤗

@ahmad-zayab
Copy link
Author

i focked the ivy repo using the github account made on another gmail but when i push the changes it goes through another github account made on an another gmail kindly solve this issue and guide me through the process

@D0m-inic
Copy link
Contributor

D0m-inic commented Aug 21, 2023

Hi, please refer to our docs https://unify.ai/docs/ivy/overview/contributing.html on contributing to ivy.

You'll need to find an open task: https://unify.ai/docs/ivy/overview/contributing/open_tasks.html#open-tasks

And go through the protocol of creating an issue for the task and linking the pull request.

As for your problems with your GitHub accounts, I would suggest forking the repo on the correct account and copying the changes across 👍 . Feel free to close your current pull request and reopen it with the correct account if necessary.

@ahmad-zayab
Copy link
Author

Hi, please refer to our docs https://unify.ai/docs/ivy/overview/contributing.html on contributing to ivy.

You'll need to find an open task: https://unify.ai/docs/ivy/overview/contributing/open_tasks.html#open-tasks

And go through the protocol of creating an issue for the task and linking the pull request.

As for your problems with your GitHub accounts, I would suggest forking the repo on the correct account and copying the changes across 👍 . Feel free to close your current pull request and reopen it with the correct account if necessary.

I did the changes you ask for kindly review it again

@D0m-inic
Copy link
Contributor

Please refer to the docs, you need to find an issue on the todo list, claim the issue and then link it to this PR

@ahmad-zayab
Copy link
Author

_ No description provided. _

Close #22205

@D0m-inic
Copy link
Contributor

Hi, why did you remove the hflip implementation and test?
Could you add it back 😃

@ahmad-zayab
Copy link
Author

@D0m-inic kindly see what's the issue here guide me to solve this

@D0m-inic
Copy link
Contributor

Hi, you're still removing the implementation of the hflip test and you've got your adjust_contrast function duplicated.

@ahmad-zayab
Copy link
Author

@D0m-inic kindly review it now.

@D0m-inic
Copy link
Contributor

D0m-inic commented Sep 3, 2023

Hi, you're still overwriting the test_paddle_vflip function and removing the comments for the title of the main and helpers. Make sure you're merging your branch correctly with master 👍

@ahmad-zayab
Copy link
Author

@D0m-inic kindly review it now

@D0m-inic
Copy link
Contributor

You are modifying the test_paddle_to_tensor test, which doesnt seem to be necessary 👍 . Also please add back the Helpers and Main headers which you have removed

@ahmad-zayab
Copy link
Author

@D0m-inic Is it right now? Kindly check it.

@D0m-inic
Copy link
Contributor

Take a look at your tests which are failing, you can run your test by using the command python -m pytest <relative_path_test_file.py>::<test_name>.

@ivy-seed ivy-seed assigned Aarsh2001 and unassigned D0m-inic Sep 18, 2023
@Aarsh2001
Copy link
Contributor

Hey @ahmad-zayab closing this PR due to inactivity, feel free to reopen once you're done with your changes 🙂

@Aarsh2001 Aarsh2001 closed this Sep 25, 2023
@ahmad-zayab
Copy link
Author

@Aarsh2001 how can I reopen this?

@ZoeCD ZoeCD reopened this Sep 29, 2023
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

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Conventional Commit PR Title

In order to be considered for merging, the pull request title must match the specification in conventional commits. You can edit the title in order for this check to pass.
Most often, our PR titles are something like one of these:

  • docs: correct typo in README
  • feat: implement dark mode"
  • fix: correct remove button behavior

Linting Errors

  • Found type "null", must be one of "feat","fix","docs","style","refactor","perf","test","build","ci","chore","revert"
  • No subject found

@ZoeCD ZoeCD linked an issue Sep 29, 2023 that may be closed by this pull request
@ZoeCD
Copy link
Contributor

ZoeCD commented Sep 29, 2023

Hey, here is a list of things I noticed you need to add/change to your PR:

  • Change the name of the PR to follow the conventional title rules
  • Add back the Helpers and Main headers
  • Check your test is passing (The workflows are running for me now, so I can't see the results yet)
  • Check you're not adding new formatting errors

I already linked the new issue you created to the PR. Also, you can read the docs here to learn more about contributing and creating your PR. We appreciate your patience!

@ahmad-zayab ahmad-zayab changed the title Master Adjusted the Image Contrast Oct 1, 2023
@ahmad-zayab
Copy link
Author

@ZoeCD kindly check it

@ahmad-zayab
Copy link
Author

@Aarsh2001 kindly check it now.

Copy link
Contributor

@Aarsh2001 Aarsh2001 left a comment

Choose a reason for hiding this comment

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

Hey @ahmad-zayab kindly fix the failing tests ? I tried running your code on my machine, it seems to fail for all backends. Can you recheck the supported_dtypes and your implementation for the function, thanks !

@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!

@ahmad-zayab
Copy link
Author

@Aarsh2001 kindly guide me on how to solve this error Check Semantic and welcome new contributors / PR Compliance Checks (pull_request_target) waiting for your reply.

@ahmad-zayab
Copy link
Author

@Aarsh2001 kindly review it

@NiteshK84
Copy link
Contributor

Hi @ahmad-zayab, all the backend tests are failing,
image
please run the test in your local env, go through the error messages which are printed above the result and try to analyse and fix your implementation of func and test, it might resolve your failures.
and let me know once it is done.
Thanks😊

@ivy-seed ivy-seed added the Stale label Feb 8, 2024
@ivy-seed
Copy link

ivy-seed commented Feb 8, 2024

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.

@NiteshK84 NiteshK84 closed this Feb 25, 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.

adjust_contrast adjust_contrast
9 participants