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

Improve math_ops.py docs #34014

Merged
merged 3 commits into from Nov 12, 2019

Conversation

stmugisha
Copy link
Contributor

No description provided.

Added usage examples to the tf.math.cumsum() docstring. What exists currently is syntax-like declarations and not  `self contained` code samples.
Added a usage example and description of the function parameters and return values
@tensorflow-bot tensorflow-bot bot added the size:S CL Change Size: Small label Nov 5, 2019
@stmugisha stmugisha closed this Nov 5, 2019
@stmugisha stmugisha reopened this Nov 5, 2019
@stmugisha stmugisha closed this Nov 5, 2019
@stmugisha stmugisha reopened this Nov 5, 2019
@stmugisha
Copy link
Contributor Author

stmugisha commented Nov 5, 2019

@yashk2810 can you help re-review this. I've cleared some commits off the log from the previous pull request (referenced above). Thanks

@rthadur rthadur self-assigned this Nov 5, 2019
@rthadur rthadur added this to Assigned Reviewer in PR Queue via automation Nov 5, 2019
@rthadur rthadur added the comp:ops OPs related issues label Nov 5, 2019
```python
tf.cumsum([a, b, c], exclusive=True) # [0, a, a + b]
```
# tf.cumsum([a, b, c], exclusive=True) => [0, a, a + b]
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a newline before this comment and everywhere else?

Something like this

By setting the `exclusive` kwarg to `True`, an exclusive cumsum is performed instead:

# tf.cumsum([a, b, c], exclusive=True)  => [0, a, a + b]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changes implemented as requested

yashk2810
yashk2810 previously approved these changes Nov 7, 2019
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Nov 7, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 7, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 7, 2019
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Nov 9, 2019
@stmugisha
Copy link
Contributor Author

The ubuntu cpu test was failing at tf.cumsum so I've made minor changes to the doctest to make it pass.

yashk2810
yashk2810 previously approved these changes Nov 11, 2019
PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Nov 11, 2019
@tensorflow-bot tensorflow-bot bot added the kokoro:force-run Tests on submitted change label Nov 11, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 11, 2019
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Nov 11, 2019
@stmugisha
Copy link
Contributor Author

stmugisha commented Nov 11, 2019

ubuntu sanity check was failing due to a pylint error(long line 82/80) and that has been fixed in the commit above.
Q: Is it by design that the tf_doctest.py test doesn't capture these?

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Nov 11, 2019
@tensorflow-bot tensorflow-bot bot added the kokoro:force-run Tests on submitted change label Nov 11, 2019
@rthadur rthadur added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Nov 11, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 11, 2019
tensorflow-copybara pushed a commit that referenced this pull request Nov 12, 2019
PiperOrigin-RevId: 279851681
Change-Id: Icf3296866d10ac23e2e2e50199a73a84cbb4becf
@tensorflow-copybara tensorflow-copybara merged commit 6f2c716 into tensorflow:master Nov 12, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Nov 12, 2019
@stmugisha stmugisha deleted the steph-en-m-patch-1 branch November 12, 2019 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:ops OPs related issues ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants