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

DOC: constants: Add constants.svg tree #11143

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

DOC: constants: Add constants.svg tree #11143

wants to merge 4 commits into from

Conversation

mkg33
Copy link
Contributor

@mkg33 mkg33 commented Nov 29, 2019

This is an example of a graphical guide embedded in the API Constants page. Basically, it is a tikzpicture converted to SVG with links added by means of a simple script (see here for more details). There have been quite a few issues with linking the tree nodes with the respective parts of the API page. Since I'm unable to build the documentation locally (seriously), I've experimented extensively with a similar layout (scipy.org) and I can confirm that the SVG file renders correctly. I'd be really grateful for any comments / general feedback.

@mkg33 mkg33 added the Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org label Nov 29, 2019
@rgommers
Copy link
Member

As you suspected @mkg33, a few fun CI failures. On TravisCI the refguide check fails with

scipy.constants:9: (WARNING/2) "raw" directive disabled.::

You should be able to reproduce that locally with $ python tools/refguide_check.py.

The doc build on CircleCI has a more informative warning for the same raw directive:

/home/circleci/repo/build/testenv/lib/python3.7/site-packages/scipy/constants/__init__.py:docstring of scipy.constants:9: WARNING: Problems with "raw" directive path:
InputError: [Errno 2] No such file or directory: '../build/testenv/lib/python3.7/site-packages/scipy/constants/constants.svg'

This is because constants.svg doesn't get installed when installing SciPy. The image file belongs under doc/, not under scipy/. You may try if this works:

.. raw:: html
   :file: ../../doc/source/_static/constants.svg

I'm not sure it will, I doubt it actually. I can't remember we have tried directly linking from a docstring to a file outside the source tree. An alternative could be to add the raw directive to doc/source/constants.rst and somehow make the image show up in the right place by moving the page header to that same .rst file.

@mkg33
Copy link
Contributor Author

mkg33 commented Nov 29, 2019

Thanks, @rgommers. I got optimistic because this setup actually worked with scipy.org documentation. I'll try to resolve this problem ASAP.

@rgommers
Copy link
Member

Built locally, the image comes out too wide and the fonts are now much larger (rather than much smaller as before) than the regular text on the page. There could be some thing related to users' font settings perhaps, not sure.

image

I'll have a look at the CI failure

@rgommers
Copy link
Member

So I think this is on purpose to make all the links inside the SVG image work, but good to note: the SVG is not a separate image, but the literal contents are inserted in the doc page:

image

@rgommers
Copy link
Member

$ python -u runtests.py -g -j2 --doc html-scipyorg reproduces the CI issue locally.

@mkg33
Copy link
Contributor Author

mkg33 commented Nov 29, 2019

Thanks for checking it, @rgommers. That's quite weird but I'll fix it. After trial and error, I managed to install/upgrade certain dependencies (to build the documentation locally) but now I'm getting this error and I've no idea how to fix it (no luck finding the answer on Google either):

Corrupt value: 0x1a00000019
Python(5655,0x10a7725c0) malloc: *** set a breakpoint in malloc_error_break to debug
make[1]: *** [html-build] Abort trap: 6
make: *** [dist] Error 2

Have you seen it before? Just to be clear: I'm trying to build it from the current cloned master (and not the broken version with my SVG addition).

As for the SVG itself: I'm aware it inserts it 'whole' and not as an image. That was the only solution that worked. In the meantime, I improved the script for inserting links and took care of exception handling.

@rgommers
Copy link
Member

Have you seen it before? Just to be clear: I'm trying to build it from the current cloned master (and not the broken version with my SVG addition).

That's not pretty. It's probably something in the build. You can try running python runtests.py. That will probably fail, in which case it's unrelated to the doc build. If that passes, then I think it's a problem related to Matplotlib.

@@ -7,6 +7,9 @@

Physical and mathematical constants and units.

.. raw:: html
Copy link
Member

Choose a reason for hiding this comment

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

Here html indicates the Sphinx "writer". So this will work (once the file is found) for the html docs, but not the pdf docs. @mkg33 I think you tried working with pdf output right? Can you remind me of the details?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much for trying to make sense of this mess. The thing about the PDF solution was this, quote from my e-mail:

So here's another solution: including the trees as pure PDFs. I've browsed the Net for similar solutions and it doesn't seem unreasonable (indeed, people have done this and apparently succeeded). Would this idea be acceptable? I have to admit, it would be great quality-wise and would facilitate maintenance. Another way would be exporting the tikzpictures as SVG - preserves the quality (though, again, not the font, but I can fix that) and is browser-friendly (in the mainstream sense).

And we later discussed the output format and you said that the SVG solution was better and that's what we'd agreed on initially. So, in a sense, I didn't even pursue the PDF solution any further. I wanted to choose PDF over SVG due to the links issue (recall that due to some unresolved bug, see the issue here, pdftocairo doesn't preserve links in the output; and, yes, I've tried multiple PDF->SVG converters and they either distort the quality/font or remove the links).

Let's wait and see what others propose with regard to this issue. I'd be very happy to work on another solution.

@rgommers
Copy link
Member

rgommers commented Nov 30, 2019

This last commit should pass CI. It may not be the optimal solution. Note that it only works for html, not for latex/pdf (the Sphinx writers after .. raw:: are the only ones used). Also, because of including the raw directive in the .rst file rather than the module docstring, it comes before rather than after the summary line of that docstring.

Perhaps a custom Sphinx directive is needed here (EDIT: something along the lines of autoimage). @larsoner or @pv, any opinion on the best way to tackle this image inclusion issue?

@rgommers
Copy link
Member

Also for reference, this is where the .. raw:: html idea came from: https://stackoverflow.com/questions/34777943/insert-clickable-svg-image-into-sphinx-documentation

@rgommers
Copy link
Member

And for some more context: the issue is that :file: within .. raw:: just wants a relative or absolute path (see http://docutils.sourceforge.net/docs/ref/rst/directives.html#raw-data-pass-through), however from within a module docstring that's included via .. automodule::, neither an absolute nor a relative path to something under /doc/source/ is well-defined. And I tried /graphical_guides as well, but Sphinx does not treat that like a path starting at the base location where conf.py is located.

@mkg33
Copy link
Contributor Author

mkg33 commented Nov 30, 2019

@rgommers regarding the build:

You can try running python runtests.py. That will probably fail, in which case it's unrelated to the doc build.

I'm getting this now:

scipy/linalg/tests/test_cython_blas.py ..Fatal Python error: Segmentation fault

And here's the end of the thread listing:

File "runtests.py", line 308 in main
File "runtests.py", line 497 in <module>
Segmentation fault: 11

To add more context to the choice of the raw with file directive. This:

.. raw:: html

    <object data="constants.svg" type="image/svg+xml"></object>

also fails to produce the output we want (there's just a blank square). The

.. image:: constants.svg

directive (.. figure:: also) inserts the tree in the right place but removes all clickable links from the picture.

Again, the above results refer to the scipy.org html documentation (but, by extension, the behaviour of the directives should be the same in scipy).

@rgommers
Copy link
Member

rgommers commented Dec 2, 2019

scipy/linalg/tests/test_cython_blas.py ..Fatal Python error: Segmentation fault

Let's separate that problem from this PR. There's something in your setup, hard to guess what. Perhaps open a new issue, and we can try and debug it together over chat/call later this week.

@rgommers
Copy link
Member

rgommers commented Dec 2, 2019

All CI failures are unrelated.

Doc build works, but links don't work very smoothly - sometimes they do, sometimes they don't. The links within the SVG look differently than the other links on the page:

<a xlink:href="#physical-constants">

for SVG vs.

<a class="reference internal" href="#physical-constants">Physical constants</a>

for Sphinx-generated links in the sidebar. Those xlink's sometimes work, because it stays on the same page. For links to a function, there needs to be a jump to another page, which doesn't work. Try for example the find function on https://16478-1460385-gh.circle-artifacts.com/0/html-scipyorg/constants.html#scipt.constants.precision

@mkg33
Copy link
Contributor Author

mkg33 commented Dec 4, 2019

@rgommers Alright, I'll move the setup issue.

As for the links, that's unexpected because I tested it with the scipy.org and everything worked fine. But I guess it's different in this situation.

I just changed it to <a class="reference internal" href=""></a>.

Could you test it? Here is the updated file:
constants.svg

@rgommers
Copy link
Member

rgommers commented Dec 4, 2019

Visually this fits better now:

image

Most links are still broken though. The ones that are links to a section on the same page work, links to other pages don't (the link format assumes everything is on that page). We'll need to find a better way to post-process the image to update the links. Each page does have the correct link in it somewhere, so that could be scraped. For example, for constants.find the link in the SVG image is

.../html-scipyorg/constants.html#scipy.constants.find

while the correct link would be

.../html-scipyorg/generated/scipy.constants.find.html#scipy.constants.find

@mkg33
Copy link
Contributor Author

mkg33 commented Dec 4, 2019

I get it now. My prototype didn't take into account the exact nature of the links because I simply had no access to the local build. It doesn't seem too difficult (or else I'm very wrong): from what I've noticed in the page source, we just need to provide the generated/... link whenever an external source is concerned and a header link if it's just about navigating within the current page.

I've modified the script so that the external pages are encoded differently (as reference internal) and the header links are encoded as headerlink.

You can review the updated solution here (lines 13-41).

And here's another test file if you still have the patience to test it for me...

@rgommers
Copy link
Member

rgommers commented Dec 5, 2019

Link almost works, it now has generated/#scipy which should be generated/scipy.

Somehow a bunch of elements disappeared:

image

@mkg33
Copy link
Contributor Author

mkg33 commented Dec 5, 2019

This is crazy, it looks completely fine in browser preview. Anyway, a silly mistake - I've changed the script accordingly and also decided to hardcode the hash so that the user won't have to type it in every line.

Here's the file.

I've opened an issue regarding my problems with building the documentation.

@rgommers
Copy link
Member

This is crazy, it looks completely fine in browser preview.

Yep, for me too - it's just messed up inside the Sphinx page.

Looking at the source, all the links of the orange boxes are fine now. The purpose boxes however aren't. Example: <a class="headerlink" href="#energy">.

@mkg33
Copy link
Contributor Author

mkg33 commented Dec 14, 2019

Correct me if I'm wrong but shouldn't the link actually look like that? This is copied verbatim from the documentation source online:

<a class="headerlink" href="#energy" title="Permalink to this headline">¶</a>

I assume that the internal reference and generated/... parts come only into play when there's a reference to an external source (i.e., not the same page) and the headerlink and href="#sth" applies to a section/paragraph on the same page. At least that's how I understood it (?)

@mkg33
Copy link
Contributor Author

mkg33 commented Jan 21, 2020

@rgommers could you perhaps try the most recent solution and see if it works on your computer? I'm still unable to set up my local build. I'm quite confident it is correct this time (I mean the hyperlinks etc.; cf. my last comment). I really want the trees to work :-)

mkg33 and others added 3 commits April 27, 2021 16:14
Testing whether the embedding of constants.svg works.
This may not be the optimal solution. Note that it only works for
html, not for latex/pdf (the Sphinx writers after ``.. raw::`` are
the only ones used).  Perhaps a custom Sphinx directive is needed here.
@rgommers
Copy link
Member

rgommers commented Apr 27, 2021

Hi @mkg33, I hope you're doing okay! We finally have an online development environment integrated via Gitpod, see http://scipy.github.io/devdocs/dev/contributor/quickstart_gitpod.html#quickstart-gitpod. That made me think of this PR. I brought it up-to-date with current master (we also have a new Sphinx theme, it's pretty now!). Gitpod should avoid all the "building SciPy is very painful" problems, you get dropped into a VS Code environment where SciPy and the docs are already built (or being built on first start up).

There's still things that need fixing, like the image width and the pdf build, but I hope this resolves the biggest blocker to work on this PR.

image

@mkg33
Copy link
Contributor Author

mkg33 commented Apr 27, 2021

Hi Ralf, it's great to hear from you! I've been thinking about this PR recently. I'm so glad that I can now try other solutions without having to deal with my local setup problems. I have 2 exams until May 15th but I'll definitely work on it afterwards. I'm really happy there's a chance to use the trees and merge this PR :-) !

@lucascolley lucascolley added the needs-work Items that are pending response from the author label Dec 23, 2023
@lucascolley lucascolley marked this pull request as draft March 14, 2024 21:32
@lucascolley lucascolley changed the title WIP: Add constants.svg tree DOC: constants: Add constants.svg tree Mar 14, 2024
@mdhaber
Copy link
Contributor

mdhaber commented Oct 1, 2024

@mkg33 Hope your exams went well! Were you still interested in finishing up this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org needs-work Items that are pending response from the author scipy.constants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants