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

Fix wrong new line #44676

Merged
merged 2 commits into from Nov 26, 2020
Merged

Conversation

albertvillanova
Copy link
Contributor

Fix wrong new line in docstring.

@google-ml-butler google-ml-butler bot added the size:XS CL Change Size: Extra Small label Nov 7, 2020
@google-cla google-cla bot added the cla: yes label Nov 7, 2020
@gbaned gbaned self-assigned this Nov 8, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Nov 8, 2020
@gbaned gbaned added the comp:ops OPs related issues label Nov 8, 2020
@gbaned gbaned requested a review from lamberta November 8, 2020 03:40
@mihaimaruseac
Copy link
Collaborator

Is there something broken? Afaik this is the markdown syntax for hyperlinks and it should not be split on two lines

@lamberta
Copy link
Member

Yeah, this change shouldn't matter. Though there is currently a forced line break on the page: https://www.tensorflow.org/api_docs/python/tf/keras/losses/Reduction?version=nightly

If we're stripping # pylint: disable=line-too-long, are we adding an extra <br>? cc: @yashk2810

@gbaned
Copy link
Contributor

gbaned commented Nov 18, 2020

@albertvillanova Any update on this PR? Please. Thanks!

@gbaned
Copy link
Contributor

gbaned commented Nov 25, 2020

@albertvillanova Can you please check @lamberta's comments and keep us posted ? Thanks!

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Nov 25, 2020
@albertvillanova
Copy link
Contributor Author

Hello @gbaned. I thought @lamberta question was addressed to @yashk2810.

Currently, there is a wrong line break in the docs: https://www.tensorflow.org/api_docs/python/tf/keras/losses/Reduction

Please see the custom training guide
for more details on this.

I have made this pull request to fix it.

Apparently, as @lamberta pointed out, current # pylint: disable=line-too-long is creating a line break, so I removed it.

@albertvillanova
Copy link
Contributor Author

albertvillanova commented Nov 25, 2020

In relation to the comment made by @mihaimaruseac, I have not invented split hyperlinks. Indeed they are already used many times in tensorflow project, e.g.:

Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

Thanks. I checked the links you provided and the documentation is not broken on the site.

TIL.

tensorflow/python/ops/losses/loss_reduction.py Outdated Show resolved Hide resolved
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 25, 2020
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Nov 25, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 25, 2020
@gbaned gbaned removed the stat:awaiting response Status - Awaiting response from author label Nov 26, 2020
@copybara-service copybara-service bot merged commit ad56abc into tensorflow:master Nov 26, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Nov 26, 2020
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:XS CL Change Size: Extra Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants