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

Fixes anchor links in installation instructions #648

Closed
wants to merge 2 commits into from
Closed

Fixes anchor links in installation instructions #648

wants to merge 2 commits into from

Conversation

fpmchu
Copy link

@fpmchu fpmchu commented Dec 29, 2015

The header anchor links in the docs (inside the directory g3doc) is using a {#anchor} format that github's Markdown does not understand. This PR primarily fixes the anchor text in the installation instructions, which I assume is fairly popular.

{#anchor} format that github's Markdown does not understand.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@fpmchu
Copy link
Author

fpmchu commented Dec 29, 2015

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@@ -30,7 +30,7 @@ images are listed in the corresponding installation sections.
If you encounter installation errors, see
[common problems](#common_install_problems) for some solutions.
Copy link
Member

Choose a reason for hiding this comment

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

There are still a couple of these left... can you look for #[a-z_]* and fix them all so we can tick off the files you've touched?

Copy link
Author

Choose a reason for hiding this comment

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

Ah sorry I missed them, let me do another round of checks then reply back.

Copy link
Author

Choose a reason for hiding this comment

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

Done. Note that this only fixes os_setup.md, and any inward reference files (so that there are no broken links). If I fix all files I touched it will quickly expand to all files :-)

@@ -121,13 +121,13 @@ $ source ~/tensorflow/bin/activate.csh # If using csh
(tensorflow)$ # Your prompt should change

# Ubuntu/Linux 64-bit, CPU only:
(tensorflow)$ pip install --upgrade https://storage.googleapis.com/tensorflow/linux/cpu/tensorflow-0.6.0-cp27-none-linux_x86_64.whl
(tensorflow)$ pip install --upgrade https://storage.googleapis.com/tensorflow/linux/cpu/tensorflow-0.5.0-cp27-none-linux_x86_64.whl
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change these to 0.5.0?

@martinwicke
Copy link
Member

the internal and OSS versions have diverged for a few days now. Also, they're different, kept in sync with a bunch of script. You can't copy any content from the third_party version into github, it won't work. You'll have to redo the change (it shoudl be possible to apply the patch to the public version you checked out from github, but no guarantees on that either).

@fpmchu
Copy link
Author

fpmchu commented Dec 30, 2015

I apologize. Let me retry this.


## Pip Installation {#pip_install}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it wise to simply remove the {#anchor} notation? From what I understand, this syntax is used internally to render the Markdown to HTML with id tags, which is then used on the tensorflow.org website.

If the goal is to have this work on both Github and tensorflow.org, my suggestion is, rather than removing the {#anchor}, replace {#old-anchor} with a {#new-anchor}, such that "#new-anchor" matches the automatically created id inside of Github.

for example, I suggest that this line should become:
## Pip Installation {#pip-installation}

@zhoubinjason
Copy link

I signed it!

2015-12-29 17:18 GMT-05:00 googlebot notifications@github.com:

Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project. Before we can look at your
pull request, you'll need to sign a Contributor License Agreement (CLA).

[image: 📝] Please visit https://cla.developers.google.com/
https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll

verify. Thanks.


Reply to this email directly or view it on GitHub
#648 (comment)
.

samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This commit does two things:

1. Removes all '{#anchor}' syntax from Markdown header lines in the codebase
2. Replaces all links that used those #anchors with links conforming to
Github/Tensorflow website auto-generated anchors

The work is a continuation on tensorflow#648.

This commit does not change the API docs, as those Markdown files are
auto-generated.

Generated using Python script available here:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/change_header_anchor_links.py
samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This changes cross-page references to outdated {#anchor} links to
instead point to the automatically created Github/TensorFlow.org anchors

See tensorflow#648
samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This commit does two things:

1. Removes all '{#anchor}' syntax from Markdown header lines in the codebase
2. Replaces all links that used those #anchors with links conforming to
Github/Tensorflow website auto-generated anchors

The work is a continuation on tensorflow#648.

This commit does not change the API docs, as those Markdown files are
auto-generated.

Generated using Python script available here:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/change_header_anchor_links.py

Cross-page links changed by hand using this script to find non-api
anchor links:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/get_anchor_references.py
samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This commit does two things:

1. Removes all '{#anchor}' syntax from Markdown header lines in the codebase
2. Replaces all links that used those #anchors with links conforming to
Github/Tensorflow website auto-generated anchors

The work is a continuation on tensorflow#648.

This commit does not change the API docs, as those Markdown files are
auto-generated.

Generated using Python script available here:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/change_header_anchor_links.py

Cross-page links changed by hand using this script to find non-api
anchor links:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/get_anchor_references.py
samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This commit does two things:

1. Removes all '{#anchor}' syntax from Markdown header lines in the codebase
2. Replaces all links that used those #anchors with links conforming to
Github/Tensorflow website auto-generated anchors

The work is a continuation on tensorflow#648.

This commit does not change the API docs, as those Markdown files are
auto-generated.

Generated using Python script available here:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/change_header_anchor_links.py

Cross-page links changed by hand using this script to find non-api
anchor links:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/get_anchor_references.py
@samjabrahams
Copy link
Contributor

Finally got my commits squashed properly, snagging all of these {#anchors}. See #660.

@martinwicke
Copy link
Member

Awesome. I didn't know about the -N uniquification. Do we have a case like
that? I'd like to check to make sure that our website behaves identically.

On Thu, Dec 31, 2015 at 1:55 PM Sam Abrahams notifications@github.com
wrote:

Finally got my commits squashed properly, snagging all of these
{#anchors}. See #660 #660.


Reply to this email directly or view it on GitHub
#648 (comment)
.

@samjabrahams
Copy link
Contributor

I haven't seen a case where it comes up in the TensorFlow docs- I just know that Github performs that action.

Also, I noticed that the code doesn't handle apostrophes or other mid-word non-alphanumeric characters correctly. Whoops! Luckily it only messed up three links, so I'll get that patched in.

I believe the change needed to make the python code work is to remove non-alphanumeric/- characters instead of replacing them with a space.

samjabrahams added a commit to samjabrahams/tensorflow that referenced this pull request Dec 31, 2015
This commit does two things:

1. Removes all '{#anchor}' syntax from Markdown header lines in the codebase
2. Replaces all links that used those #anchors with links conforming to
Github/Tensorflow website auto-generated anchors

The work is a continuation on tensorflow#648.

This commit does not change the API docs, as those Markdown files are
auto-generated.

Generated using Python script available here:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/change_header_anchor_links.py

Cross-page links changed by hand using this script to find non-api
anchor links:
https://github.com/samjabrahams/tensorflow_util/blob/master/py/get_anchor_references.py
@samjabrahams
Copy link
Contributor

Those three goofed links in #660 have been fixed. Also, I modified the anchor-creating Python function back in the line comments to work properly. Just had to change the first lambda to lambda x: ""

@samjabrahams
Copy link
Contributor

After realizing I missed some details for the GitHub-ification of header anchors, I played around with more edge cases to hone the algorithm. I had to make three small changes to what I had above:

  1. GitHub does not remove trailing hyphens
  2. It replaces groups of whitespace with hyphens before removing strange characters.
    • This can lead to trailing hyphens. e.g. "This header ends with ellipses ..." -> "#this-header-ends-with-ellipses-"
  3. Make sure that the regex substitution is flagged for UTF-8 encoding

GitHub handles UTF-8 encoding in its header anchors, so make sure that any settings necessary to enable UTF-8 are switched on.

I changed the code in the line notes above, but here is the fixed Python code again so going back up to read it isn't necessary. To test out Unicode letters in Python, make sure to explicitly make the string unicode with u"This is my héader string!" syntax:

def create_anchor_from_header(header, existingAnchors=None):

    # Strip white space on the left/right and make lower case
    out = header.strip().lower()

    # Replace groups of white space with hyphens
    out = re.sub(ur'\s+', lambda x: "-", out, flags=re.UNICODE)

    # Remove non-alphanumeric characters, hyphens, and spaces
    out = re.sub(ur'[^\w\- ]+', lambda x: "", out, flags=re.UNICODE)

    if existingAnchors:
        if out in existingAnchors:
            i = 1
            while (out + "-" + str(i)) in existingAnchors and i < 100:
                i = i + 1
            out += "-" + str(i)
    return out

@martinwicke
Copy link
Member

This PR is superceded by #660, I'll close it.

@martinwicke martinwicke closed this Jan 4, 2016
@fpmchu
Copy link
Author

fpmchu commented Jan 5, 2016

I'm late to this. Thanks @samjabrahams for taking this and making it much better :-)
I wrote a small one to fix up some issues I found:
#683
Not sure if there are other like this.

darkbuck pushed a commit to darkbuck/tensorflow that referenced this pull request Jan 23, 2020
…pstream-zyin-workaround-hip-unloadmodule

Adding dockerfile workaround for hip runtime issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants