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 usage examples to some APIs #35388

Merged

Conversation

@msteknoadam
Copy link
Contributor

msteknoadam commented Dec 24, 2019

Added usage examples to these APIs

  • image.random_flip_up_down
  • image.flip_up_down
  • image.random_flip_left_right
  • image.flip_left_right
  • image.transpose
  • image.random_brightness
  • image.random_contrast
  • image.random_hue
  • image.random_jpeg_quality
  • image.random_saturation

-- Re-Opened the PR since the last one wasn't based off the master --

msteknoadam and others added 5 commits Dec 22, 2019
Added to:
- image.random_flip_up_down
- image.flip_up_down
- image.random_flip_left_right
- image.flip_left_right
Added usage examples to these APIs aswell:
- image.transpose
- image.random_brightness
- image.random_contrast
- image.random_hue
- image.random_jpeg_quality
- image.random_saturation
Co-Authored-By: Kilaru Yasaswi Sri Chandra Gandhi <yasaswisrichandragandhi@gmail.com>
…o example-adding-branch
@tensorflow-bot tensorflow-bot bot added the size:M label Dec 24, 2019
@googlebot

This comment has been minimized.

Copy link

googlebot commented Dec 24, 2019

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no label Dec 24, 2019
@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Dec 24, 2019

@googlebot I consent.

@kyscg

This comment has been minimized.

Copy link
Contributor

kyscg commented Dec 25, 2019

@googlebot I consent

@googlebot

This comment has been minimized.

Copy link

googlebot commented Dec 25, 2019

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Dec 25, 2019
@ravikyram ravikyram self-assigned this Dec 26, 2019
@ravikyram ravikyram added this to Assigned Reviewer in PR Queue via automation Dec 26, 2019
@ravikyram ravikyram added the comp:ops label Dec 26, 2019
@rthadur rthadur requested a review from mihaimaruseac Dec 26, 2019
@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Dec 26, 2019

Yeah actually that idea feels good, I will do it ASAP. Thanks for the recommendation!

@mihaimaruseac

This comment has been minimized.

Copy link
Collaborator

mihaimaruseac commented Dec 26, 2019

Thanks for the update. However, it still fails to be testable.

I was thinking of something more on the lines of

>>> t = [[1, 2, 3],
... [4, 5, 6]]
>>> tf.reshape(t, [3, 2]).numpy()
array([[1, 2],
[3, 4],
[5, 6]], dtype=int32)
>>> tf.transpose(t, perm=[1, 0]).numpy()
array([[1, 4],
[2, 5],
[3, 6]], dtype=int32)

@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Dec 26, 2019

Hmm yeah, ok. I will be changing them in a few mins.

@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Dec 26, 2019

Ok, made the changes. Can you check again?

@mihaimaruseac

This comment has been minimized.

Copy link
Collaborator

mihaimaruseac commented Dec 26, 2019

Looks better but still not there:

  • Don't use tf.random.normal as then testing would depend on the seed that is being used. Instead, explicitly use some values (like in the example above, t = [[1, 2, 3],...)
  • Use >>> instead of >> as >>> signal that this is documentation test which will then be tested.
  • There is no need for the triple backquote markers and the python tagging. The >>> makes this look like properly-formatted code in the documentation.
  • Don't insert these examples after the typing/argument documentation, but before that. Args:/Returns:/etc. are expected to always be the last blocks in a docstring.
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
tensorflow/python/ops/image_ops_impl.py Outdated Show resolved Hide resolved
PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes Dec 27, 2019
@mihaimaruseac

This comment has been minimized.

Copy link
Collaborator

mihaimaruseac commented Jan 7, 2020

I think you might need to rebase against master again

…o example-adding-branch
@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Jan 7, 2020

Sure thing, done!

@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Jan 7, 2020

By the way, I guess last error was a false-triggered error since it was giving 404 when reaching to logs and I have seen that when you re-ran it, it didn't give an error.

@rthadur

This comment has been minimized.

Copy link
Contributor

rthadur commented Jan 7, 2020

@msteknoadam can you please resolve conflicts

PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Jan 7, 2020
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jan 7, 2020
@rthadur rthadur added ready to pull and removed ready to pull labels Jan 7, 2020
tensorflow-copybara pushed a commit that referenced this pull request Jan 7, 2020
PiperOrigin-RevId: 288567304
Change-Id: I92da0849729e645ceab19a5791737ea20e3d7f12
@tensorflow-copybara tensorflow-copybara merged commit 24a3a51 into tensorflow:master Jan 7, 2020
8 of 10 checks passed
8 of 10 checks passed
Windows Bazel Internal CI build failed
Details
Windows Bazel GPU Internal CI build failed
Details
Android Demo App Internal CI build successful
Details
Linux GPU Internal CI build successful
Details
MacOS CPU Python3 Internal CI build successful
Details
MacOS Python2 and CC Internal CI build successful
Details
Ubuntu CPU Internal CI build successful
Details
Ubuntu Sanity Internal CI build successful
Details
cla/google All necessary CLAs are signed
import/copybara Change imported to the internal review system
Details
PR Queue automation moved this from Approved by Reviewer to Merged Jan 7, 2020
@msteknoadam

This comment has been minimized.

Copy link
Contributor Author

msteknoadam commented Jan 8, 2020

Sorry, I was sleeping at the moment you requestef for changes but thanks for doing the changes for me. I'm very happy that this PR finally got merged :D Have a great day everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.