photutils icon indicating copy to clipboard operation
photutils copied to clipboard

Source code link error

Open PrasannaJayanthi opened this issue 4 years ago • 16 comments

I was looking at the tutorials on the photutils website and there is an error in the source code link. There are two slashes instead of one. This is the current link "https://photutils.readthedocs.io/en/stable//background-1.py", it works if the link is this "https://photutils.readthedocs.io/en/stable/background-1.py". I am assuming the issue is the same for all the source code links.

PrasannaJayanthi avatar Apr 06 '20 15:04 PrasannaJayanthi

Thanks for the report.

It's an interesting plot:: directive issue. Interestingly I don't see it in another package, but indeed it's persistently present in photutils.

@larrybradley @pllim - would you think that we can linkcheck for these internal docs links, too?

bsipocz avatar Apr 06 '20 16:04 bsipocz

I am not familiar with the code here. Where is that code generated?

pllim avatar Apr 06 '20 16:04 pllim

Those links are dynamically generated by Sphinx, specifically the sphinx.ext.viewcode extension. I just noticed that astropy turned off showing the source code by setting plot_html_show_source_link = False in docs/conf.py.

larrybradley avatar Apr 06 '20 16:04 larrybradley

But where is it generated? Which RST file? 😬

pllim avatar Apr 06 '20 16:04 pllim

You can pick any of them. It applies to all plot:: directives. @PrasannaJayanthi report specifically refers to https://github.com/astropy/photutils/blob/master/docs/background.rst

larrybradley avatar Apr 06 '20 16:04 larrybradley

It's definitely an upstream issue. specutils has the same issue, e.g. try the "Source code" link above the first plot on this page: https://specutils.readthedocs.io/en/latest/

larrybradley avatar Apr 06 '20 16:04 larrybradley

Yeah, definitely, I noticed that in synphot as well. 🤔

pllim avatar Apr 06 '20 16:04 pllim

Yes, it's certainly upstream, but I don't yet see where it's coming from. Weirdly I don't see it with astroML while messing with the new docs here: https://epyc.astro.washington.edu/~bsipocz/astroml/regression/_build/html/user_guide/regression.html (while it's annoying there, that I also get the "source code" links for code snippets even though I didn't asked for it.)

bsipocz avatar Apr 06 '20 17:04 bsipocz

Hmm, maybe it's in sphinx-astropy? 🤔

larrybradley avatar Apr 06 '20 17:04 larrybradley

Good point, though I don't yet see how, as we don't patch the plot directive there.

maybe @astrofrog has insights.

bsipocz avatar Apr 06 '20 17:04 bsipocz

I don't see how this can add extra slash...

https://github.com/astropy/sphinx-astropy/blob/088f073519a02bd02a9c38dc8b14d81dcc4e69ad/sphinx_astropy/conf/v1.py#L138-L140

try:
    import matplotlib.sphinxext.plot_directive
    extensions += [matplotlib.sphinxext.plot_directive.__name__]

@larrybradley mentioned maybe it was

https://github.com/sphinx-doc/sphinx/blob/f6882d74663438b281a1260684a16e97696ed6d9/sphinx/ext/viewcode.py#L193

                parents.append({
                    'link': urito(pagename, '_modules/' +
                                  parent.replace('.', '/')),

which leads to

https://github.com/sphinx-doc/sphinx/blob/f6882d74663438b281a1260684a16e97696ed6d9/sphinx/util/osutil.py#L58

but hard to tell where the bug is without knowing the values of from (or base) and to strings.

pllim avatar Apr 06 '20 21:04 pllim

oh, that indeed looks like to be it!

Does either of you digging into it for a fix, or shall I?

bsipocz avatar Apr 07 '20 01:04 bsipocz

@bsipocz , I have not gotten to it yet and unlikely this week, so feel free to take it. Thanks! 🙇‍♀

pllim avatar Apr 07 '20 14:04 pllim

Sounds good, this will be my rewards project this week then 😅

bsipocz avatar Apr 07 '20 15:04 bsipocz

BTW, on first glance it appeared that was the bug (https://github.com/sphinx-doc/sphinx/blob/f6882d74663438b281a1260684a16e97696ed6d9/sphinx/ext/viewcode.py#L193), but I briefly tried "fixing" it and nothing change. The bug may be elsewhere.

Also, I noticed that the PNG, PDF, etc. links also have double slashes in the URL (if you look at the HTML source code). But those links work somehow.

larrybradley avatar Apr 07 '20 15:04 larrybradley

Still should be avoided regardless. I think it becomes an issue if there is some sort of ambiguity in resolving the server-side contents.

https://stackoverflow.com/questions/20523318/is-a-url-with-in-the-path-section-valid

pllim avatar Apr 07 '20 15:04 pllim