search

spatialaudio / nbsphinx

:ledger: Sphinx source parser for Jupyter notebooks
https://nbsphinx.readthedocs.io/
MIT License
451 stars 130 forks source link

Add footcite feature #536

Closed clonker closed 3 years ago

clonker commented 3 years ago

Hi, as mentioned in issue #535, this adds the data-footcite attribute to the CitationParser. One could check if there are both data-cite and data-footcite present inside the HTML tag, however I think that is a fairly exotic case. In this implementations' current state, the attribute that comes first would simply take precedence. I took the liberty to update the documentation notebooks accordingly. Please let me know if you have any suggestions!

Source:

footbibliography_source

Result: footbibliography_result

clonker commented 3 years ago

I am not sure about the CI failures but they seem unrelated to this proposed set of changes

mgeier commented 3 years ago

Thanks for this PR! I will have a closer look soon.

Rendered: https://nbsphinx--536.org.readthedocs.build/en/536/markdown-cells.html#Citations

Is this expected to not work with LaTeX/PDF output?

See: https://632-46379698-gh.circle-artifacts.com/0/nbsphinx.pdf#subsection.3.2

If that's expected, can you please mention that in the docs?

I am not sure about the CI failures but they seem unrelated to this proposed set of changes

Yeah, I think they are unrelated. The linkcheck error was just temporary, but the other error is related to sphinxcontrib-bibtex, I will look into that later.

clonker commented 3 years ago

Interesting, I hadn't checked the PDF output. It seems to me that the example citation was correctly placed into the footnotes - just the directive itself didn't receive any specific output. Perhaps it should be mentioned more explicitly that this does not render a bibliography in the classical sense but rather uses footnotes, especially in LaTeX/PDF output. Anyways it seems a bit odd to me that the doi got another footnote within the footnote (ie footnotes 134 and 135). I am not sure right now whether this is expected behavior and/or caused by sphinxcontrib-biblatex or by the way things are extracted from the ntoebook.

clonker commented 3 years ago

So the double footnote I was worried about is caused by the latex_show_urls = 'footnote' option in conf.py. The same effect with raw reST files and just sphinxcontrib-biblatex, so this is somewhat expected behavior. What would you say if I added a remark to the documentation that the directive only renders as bibliography in html output, in LaTeX/PDF output it causes the footnote citations to be placed into the footnotes depending on the page they're on?

mgeier commented 3 years ago

Yes, the additional footnote is strange, and it would be nice to get rid of it, but it isn't the fault of this PR.

It seems to me that the example citation was correctly placed into the footnotes - just the directive itself didn't receive any specific output.

Oh, yes, I didn't see the footnote.

Perhaps it should be mentioned more explicitly that this does not render a bibliography in the classical sense but rather uses footnotes, especially in LaTeX/PDF output.

Yes, please.

What would you say if I added a remark to the documentation that the directive only renders as bibliography in html output, in LaTeX/PDF output it causes the footnote citations to be placed into the footnotes depending on the page they're on?

Yes please!

There are already a few things that are different in HTML and LaTeX output. That's fine, but I'd like to mention all differences in the docs. I've also mentioned the different behavior there: https://nbsphinx.readthedocs.io/en/0.8.1/a-normal-rst-file.html#references

mgeier commented 3 years ago

I've come up with a different way to use this, without needing raw reST cells:

nbsphinx_epilog = r"""
.. footbibliography::
"""

What do you think about this?

It's probably not appropriate to use this in the nbsphinx docs, but you could probably mention it?

clonker commented 3 years ago

I'll get on it then, thank you for the feedback :slightly_smiling_face: I think the epilog is a brilliant idea, makes the whole thing so much cleaner and easier to use, for sure worth mentioning!

clonker commented 3 years ago

I think I have incorporated all the suggestions, please let me know if there is anything that I missed or that can be improved.

mgeier commented 3 years ago

Thanks for the update!

I think it might be less confusing if the new footcite feature gets a separate section in the "Markdown Cells" notebook. Either as sub-section of "Citations" or as sibling section right after it. What do you think?

The structure on the "normal reST files" page is also a bit confusing: There is a "citations" section followed by "references", which then includes some text about (foot) "citations" again.

AFAIU, bibtex_bibfiles is needed for both "normal" and "foot" citations, right?

So probably it would make sense to start with a "references" section explaining the use of bibtex_bibfiles and then add two sub-sections "Citations" and "Foot Citations", or something like that?

clonker commented 3 years ago

I think it might be less confusing if the new footcite feature gets a separate section in the "Markdown Cells" notebook. Either as sub-section of "Citations" or as sibling section right after it. What do you think?

That would help with clarity, I was just reluctant to make bigger changes to the documentation. Since it is a special case of citations perhaps a sub-section makes here more sense than sibling section.

The structure on the "normal reST files" page is also a bit confusing: There is a "citations" section followed by "references", which then includes some text about (foot) "citations" again. AFAIU, bibtex_bibfiles is needed for both "normal" and "foot" citations, right? So probably it would make sense to start with a "references" section explaining the use of bibtex_bibfiles and then add two sub-sections "Citations" and "Foot Citations", or something like that?

Fully agree, let me make an update to this pr tomorrow and we'll take it from there.

clonker commented 3 years ago

I restructured the document as you suggested and in the process removed the reference to the unmaintained sphinx-natbib extension. The bitbucket repository seems to be gone and there is just one github fork of it with last commit 8 years ago. If you think it should remain in the docs regardless I will re-add it of course. What do you think?

clonker commented 3 years ago

Thank you for your remarks, I hope everything is in order now - but of course I am happy to incorporate further changes! I am not sure how your policy is with respect to the git log, if you like I can squash the commits into one.

mgeier commented 3 years ago

I am not sure how your policy is with respect to the git log, if you like I can squash the commits into one.

Unless the changes are intentionally split into multiple meaningful commits, I normally just squash them when merging.

If you want, you can squash them, but if not it isn't a problem for me to squash them ...

clonker commented 3 years ago

The "bibliography directive" now links to the normal rst file "footnote citations" subsection. If it is not a problem for you to squash them then I'll leave that to you :slightly_smiling_face:

mgeier commented 3 years ago

Perfect, thanks a lot!

clonker commented 3 years ago

Also thanks to you for working with me on this, it'll come in very handy in my project(s)! 🎉