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

Added rgb_to_grayscale #19393

Merged
merged 13 commits into from
Apr 11, 2024
Merged

Added rgb_to_grayscale #19393

merged 13 commits into from
Apr 11, 2024

Conversation

IMvision12
Copy link
Contributor

@IMvision12 IMvision12 commented Mar 28, 2024

keras/backend/torch/image.py Outdated Show resolved Hide resolved
keras/ops/image.py Outdated Show resolved Hide resolved
keras/ops/image_test.py Show resolved Hide resolved
@IMvision12 IMvision12 requested a review from fchollet March 28, 2024 20:54
@IMvision12
Copy link
Contributor Author

Hi @fchollet Thanks for the initial review, can you tell me how can i fix failing tests?

@fchollet
Copy link
Collaborator

There seems to be more than one issue with the unit tests. For the particular issue you mentioned, I think you just need to make sure to call super().__init__(). Basically just do the same as what the other ops are doing and it will work.

@IMvision12
Copy link
Contributor Author

IMvision12 commented Apr 10, 2024

@fchollet can you review?

@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2024

Codecov Report

Attention: Patch coverage is 74.71264% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 76.24%. Comparing base (cbb64ab) to head (98e587d).
Report is 86 commits behind head on master.

Files Patch % Lines
keras/ops/image.py 63.63% 4 Missing and 4 partials ⚠️
keras/backend/torch/image.py 73.68% 3 Missing and 2 partials ⚠️
keras/backend/jax/image.py 81.25% 1 Missing and 2 partials ⚠️
keras/backend/numpy/image.py 81.25% 1 Missing and 2 partials ⚠️
keras/backend/tensorflow/image.py 78.57% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19393      +/-   ##
==========================================
+ Coverage   75.95%   76.24%   +0.28%     
==========================================
  Files         366      367       +1     
  Lines       40707    41161     +454     
  Branches     7934     8051     +117     
==========================================
+ Hits        30920    31384     +464     
+ Misses       8074     8057      -17     
- Partials     1713     1720       +7     
Flag Coverage Δ
keras 76.10% <74.71%> (+0.29%) ⬆️
keras-jax 60.24% <33.33%> (+0.14%) ⬆️
keras-numpy 54.17% <34.48%> (+0.12%) ⬆️
keras-tensorflow 61.51% <31.03%> (+0.11%) ⬆️
keras-torch 60.31% <34.48%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the update -- the code looks good overall, however the tests are still failing for two backends with shape correctness issues. Please take a look.

keras/ops/image.py Outdated Show resolved Hide resolved
@IMvision12 IMvision12 requested a review from fchollet April 11, 2024 09:42
Copy link
Collaborator

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@google-ml-butler google-ml-butler bot added kokoro:force-run ready to pull Ready to be merged into the codebase labels Apr 11, 2024
@fchollet fchollet merged commit 2fb6a2a into keras-team:master Apr 11, 2024
6 checks passed
@google-ml-butler google-ml-butler bot removed awaiting review ready to pull Ready to be merged into the codebase kokoro:force-run labels Apr 11, 2024
@IMvision12 IMvision12 deleted the gray branch April 11, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

4 participants