Skip to content

Fix Astropy intersphinx URL - #1841

Merged
astrofrog merged 1 commit into
astropy:masterfrom
astrofrog:docs/fix-intersphinx
Dec 11, 2013
Merged

astrofrog merged 1 commit into
astropy:masterfrom
astrofrog:docs/fix-intersphinx

Conversation

@astrofrog

Copy link
Copy Markdown
Member

As reported by @Cadair, intersphinx doesn't work, because the URL is incorrect.

@Cadair

Cadair commented Nov 29, 2013

Copy link
Copy Markdown
Member

Thanks :)

@astrofrog

Copy link
Copy Markdown
Member Author

By the way, does it make sense to use stable or latest here?

@Cadair

Cadair commented Nov 29, 2013

Copy link
Copy Markdown
Member

I would say stable, you can always override to latest?

@astrofrog

Copy link
Copy Markdown
Member Author

Ok, makes sense.

@mdboom

mdboom commented Nov 29, 2013

Copy link
Copy Markdown
Contributor

Wouldn't it be better to handle this with a redirect (as docs.python.org does), rather than putting the burden on the user to remember our versioning scheme (which may change in the future, however unlikely). For example, http://docs.python.org/api/index.html (i.e. a deep link into the docs) redirects to http://docs.python.org/2/api/index.html. We seem to have a redirect from docs.astropy.org to docs.astropy.org/en/latest, but it doesn't work with deeper links within that tree. I don't know if that's possible with our DNS/readthedocs etc. infrastructure, but I think that's a much cleaner solution to this problem, and it keeps the URLs consistent with other projects we intersphinx link to.

@astrofrog

Copy link
Copy Markdown
Member Author

@mdboom - the redirects that are in place were all set up by RTD, and I'm not sure if we can add a redirect for intersphinx.

@mdboom

mdboom commented Nov 29, 2013

Copy link
Copy Markdown
Contributor

According to readthedocs/readthedocs.org#285, you can do: http://docs.astropy.org/page/objects.inv, but it's questionable whether http://docs.astropy.org/page/ is any better than http://docs.astropy.org/en/stable/.

There is also this pull request, readthedocs/readthedocs.org#544, merged just 4 days ago and apparently not yet "live", that does fix the redirects to do what we need once and for all. So I'd be in favor of copying the objects.inv file to astropy.github.com as a temporary measure (which will allow existing affiliated packages to continue to work as they do and always have), and waiting for this PR to go live, at which point we change the intersphinx url to http://docs.astropy.org/.

@astrofrog

Copy link
Copy Markdown
Member Author

@mdboom - ok, I'll copy the objects.inv for 0.3 to astropy.github.com. I'll leave this open to remind us to change the URL to docs.astropy.org.

@eteq

eteq commented Nov 29, 2013

Copy link
Copy Markdown
Member

@astrofrog - I'm not sure we want to add objects.inv to the web site. that might give people the impression they should use astropy.org (or astropy.github.io), when I would say it's a bug that we did that (I think it was me, and it was before we had even set up the docs URLs). If we put it in now, then people will think that it's the right thing to link through, but it will then always be v0.3 that they link against - a confusing failure mode.

Hence, I think it might be safer if we merge this as-is, and update it to /page/ when that becomes possible. It's not that big a deal to use the specific URL at least temporarily - after all, that was the whole point of the RTD "stable" build - to provide a semi-permanent location for specific pages.

@mdboom - thanks for pointing out readthedocs/readthedocs.org#544 - that will make our lives a lot easier for a variety of things. Hope it goes live soon! Once it does we should be able to start adjusting various links to use page instead of pointing to a specific version. We may even want to consider eliminating stable as an RTD build after that.

@mdboom

mdboom commented Nov 29, 2013

Copy link
Copy Markdown
Contributor

@eteq: Just to be clear, page is already possible. #544 adds support for anything at the top-level redirecting to stable.

@eteq

eteq commented Nov 29, 2013

Copy link
Copy Markdown
Member

Oh, I see - I got the two PRs confused. Yes, it is definitely preferable to do it the top-level way once that one goes live. So adjust my comment above to that instead of using page.

@toffer

toffer commented Dec 9, 2013

Copy link
Copy Markdown

I'm the author of the ["Refactor URL Routing" PR][https://github.com/readthedocs/readthedocs.org/pull/544] mentioned above, so I wanted to clarify what it does and doesn't do, and see if I can help you figure out the best way to handle intersphinx linking.

The changes I made to the redirect routing for ReadtheDocs (RTD) were mostly an internal refactoring, and don't (with a few small exceptions) change the how they work on the site, so I'm afraid the redirects don't quite work how you are hoping they do.

It's true that http://docs.astropy.org/ redirects to http://docs.astropy.org/en/stable/, but that only works for the "top-level" link, not for internal pages. It's designed to redirect people from http://docs.astropy.org/ to the default version of your documentation, since serving up a 404 here would be a pretty terrible user experience. (If your "latest" branch was designated as your default version, then it would redirect to http://docs.astropy.org/en/latest.) But, it's not a universal redirecting solution. So, as you've already noticed, a link to an internal page like http://docs.astropy.org/constants/ doesn't redirect to http://docs.astropy.org/en/stable/constants/. It didn't work that way before my change, and it doesn't work that way after my change.

The reasoning behind this is that RTD organizes the URLs for docs so that multiple translations and multiple versions of your docs can be organized logically and consistently for all projects that RTD hosts. For the way that RTD views docs, http://docs.astropy.org/en/stable/ is the root directory for your stable documentation in English, not http://docs.astropy.org/. Just like http://docs.astropy.org/en/latest/ is the root for your development documentation in English (taken from the "master" branch in git, which you've designated as the "latest" version in RTD.)

Among all the multiple versions of docs, you can choose which is the "default" version for RTD to display, which usually corresponds to the git branch of the most recent official release from your project. In the case of astropy, the "stable" branch is designated as the default.

Given that, I think it makes the most sense to set your intersphinx URL to http://docs.astropy.org/en/stable/, as was done in @astrofrog's fix above.

Another option that @mdboom mentioned above, it to use http://docs.astropy.org/page/ as your root directory, since /page links are designed to handle internal redirects to the equivalent link in your default version. For example, http://docs.astropy.org/page/constants/ does redirect to http://docs.astropy.org/en/stable/constants/. Perhaps this would be better if you are concerned about changing the name of your default version down the road from "stable" to something else. But, if you foresee "stable" always being the default branch, then I think you should stick with /en/stable, rather than /page.

Hopefully this explanation is helpful. Please let me know if you have any questions/concerns!

cc @ericholscher

@astrofrog

Copy link
Copy Markdown
Member Author

@toffer - thanks for the explanation! so just to make sure I understand, what did readthedocs/readthedocs.org#544 do - was it mainly internal refactoring? Or did it change any forwarding behavior?

@ericholscher

Copy link
Copy Markdown

Correct. We are just cleaning up our existing redirect logic. I have tried to take some of @toffer's comment up there and document it, to avoid further confusion: http://docs.readthedocs.org/en/latest/redirects.html

Let me know if you have any other questions. I believe both linking to /en/stable/ or /page/ would work. The /page/ redirect gives you a bit more versatility in the future, if you want to change the version you call 'stable'.

@mdboom

mdboom commented Dec 10, 2013

Copy link
Copy Markdown
Contributor

Thanks for the detailed response, and sorry I misinterpreted what was being done over at readthedocs.org. That all makes sense.

In light of this, I think linking to astropy.org/en/stable/objects.inv makes the most sense. It's a shame that this is inconsistent with the other Python projects we intersphinx link to, but rtd's approach is certainly rational.

@ericholscher

Copy link
Copy Markdown

Yea. The other option you have is you could point your 'latest' version to the 'stable' branch. Some people use RTD this way, but generally 'latest' points to your latest commit, not latest stable release. You could then just build your 'master' branch. I'm not really sure which is better, but the way you have it is the way I envisioned things working :)

@astrofrog

Copy link
Copy Markdown
Member Author

I think it makes sense to use stable - I don't think we want to change the meaning of latest at this stage.

Thanks @ericholscher and @toffer!

@astrofrog

Copy link
Copy Markdown
Member Author

The Travis failure is unrelated.

astrofrog added a commit that referenced this pull request Dec 11, 2013
@astrofrog
astrofrog merged commit c1abe58 into astropy:master Dec 11, 2013
@mwcraig mwcraig mentioned this pull request May 7, 2014
2 tasks done
@astrofrog
astrofrog deleted the docs/fix-intersphinx branch July 5, 2016 18:46
astrofrog added a commit to astropy/sphinx-astropy that referenced this pull request Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants