♻️ REFACTOR: package code

[chrisjsewell]

Essentially re-writes the most of the code, to implement a number of improvements and bug fixes, but the output LaTeX is shown (via tests) to be essentially the same.

• move event functions to separate file (and rename)
• add sphinx based tests, which provide comparisons of the doctrees create with/without this extension, before/after post-transforms have been applied. This also shows how the package can be used as a "standalone" sphinx extension.
• use importlib.resources for accessing jupyterBook.cls
• Remove enforcement of myst-parser amsmath extension
• make myst_nb requirement optional, and check for compatibility
• convert class/function nameing from pascalCase to camel_case (and enforce with pep8-naming linter):
  • replace getFilenameWithSubpath with is_root_document
  • replace removeExtension with remove_suffix
• replace H2Node and H3Node by RootHeader node, which applies to any sub-section of the root file (i.e. also H4 etc)
• added app.add_config_value("jblatex_load_imgconverter", True, "env") to enable loading of sphinx.ext.imgconverter to be turned off
• remove reliance on _toc.yml, instead adding app.add_config_value("jblatex_captions_to_parts", None, "env", (type(None), bool))
  • If None and app.env.external_site_map is present (from sphinx-external-toc), then this will be used to "infer" latex_toplevel_sectioning and whether to convert captions to parts.
• Move all doctree restructuring to the post-transform, so that the stored doctrees are builder agnostic (as should be the case). The transform (which is now called for all builders) only adds extra node attributes for use by the post-transform.
• a lot more code documentation and added to README

[chrisjsewell]

Not finished, but publishing so @mmcky / @AakashGfude don't do any conflicting work

TODO: rewrite LatexMasterDocTransforms, ToctreeTransforms, handleSubSections, add more testing

[codecov]

Codecov Report

Merging #55 (1e3485c) into master (c424447) will decrease coverage by 6.32%.
The diff coverage is 89.16%.

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
- Coverage   96.32%   90.00%   -6.33%     
==========================================
  Files           4        4              
  Lines         245      240       -5     
==========================================
- Hits          236      216      -20     
- Misses          9       24      +15     

Flag	Coverage Δ
pytests	90.00% <89.16%> (-6.33%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
jupyterbook_latex/events.py	86.66% <86.66%> (ø)
jupyterbook_latex/transforms.py	88.05% <88.23%> (-7.63%)	⬇️
jupyterbook_latex/__init__.py	93.33% <93.33%> (-2.13%)	⬇️
jupyterbook_latex/nodes.py	97.82% <95.83%> (-2.18%)	⬇️

[mmcky]

@AakashGfude there has been a lot of discussion over the past few days re: sphinx-external-toc so let's catchup this morning on zoom and I will bring you up to speed.

[chrisjsewell]

ok this is ready to review, one TODO to consider in the initial comment

[mmcky]

thanks @chrisjsewell -- we will review this today.

[mmcky]

@AakashGfude this PR should be reviewed against this version of sphinx-external-toc

[mmcky]

Thanks @chrisjsewell on your work on this refactor. It is a big improvement. We will apply some of these changes (in style etc.) to sphinx-multitoc-numbering as well.

I re-ran the full tests of building our lecture sites for both html and pdf (using this refactor):

• MAINT: Adjustments for juptyer-book=0.11.1 for sphinx-eternal-toc changes QuantEcon/lecture-python-programming.myst#135 generates the pdf version nicely
• MAINT: Updates for new release of juptyer-book=0.11.1 QuantEcon/lecture-python.myst#133 fails to build the pdf giving the following extension error. It seems to be related to myst_nb?

Extension error:
Handler <function validate_config_values at 0x7fb649c8e550> for event 'config-inited' threw an exception (exception: entry_points() got an unexpected keyword argument 'group')
Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/quantecon/lib/python3.8/site-packages/sphinx/events.py", line 110, in emit
    results.append(listener.handler(self.app, *args))
  File "/usr/share/miniconda3/envs/quantecon/lib/python3.8/site-packages/myst_nb/__init__.py", line 256, in validate_config_values
    load_renderer(app.config["nb_render_plugin"])
  File "/usr/share/miniconda3/envs/quantecon/lib/python3.8/site-packages/myst_nb/render_outputs.py", line 80, in load_renderer
    eps = entry_points(group="myst_nb.mime_render", name=name)
TypeError: entry_points() got an unexpected keyword argument 'group'

the build logs are here and the artifact is a complete _build folder (cc: @AakashGfude).

[mmcky]

thanks @chrisjsewell I confirm both of our current project tests cases are now building via CI

[mmcky]

@chrisjsewell nice work!

[AakashGfude]

Awesome work @chrisjsewell . The only thing left is to transfer tableofcontents directive here, for which I will create an issue and PR. Thanks so much.

[chrisjsewell]

Thanks, I'm not sure what you mean by transfer the tableofcontents directive though, that's part of sphinx-external-toc now, you wouldn't use it here. If you want those bullet lists of contents (which should be optional, with "pure" latex alternatives also made available) you'd just be looking for non-hidden toctrees

[chrisjsewell]

Also, just to clarify, I'll merge this once I've finished the final point

[AakashGfude]

Thanks, I'm not sure what you mean by transfer the tableofcontents directive though, that's part of sphinx-external-toc now, you wouldn't use it here. If you want those bullet lists of contents (which should be optional, with "pure" latex alternatives also made available) you'd just be looking for non-hidden toctrees

ooh. I could not see the tableofcontents directive result in the pdf, so assumed it was not handled for latex yet.

A screenshot:

book.pdf

[chrisjsewell]

The tableofcontents now works no different to if you were just using a regular toctree (without sphinx-external-toc).
So the point is you should be thinking of a "generic" solution that would work with any regular sphinx build (as the rest of the package now does)

[AakashGfude]

The tableofcontents now works no different to if you were just using a regular toctree (without sphinx-external-toc).
So the point is you should be thinking of a "generic" solution that would work with any regular sphinx build (as the rest of the package now does)

Sorry, maybe I did not make my point clear earlier. The problem I faced when using a regular toctree with tableofcontents like the rest of the sphinx build, was that in the case of latex, it literally included the files itself, instead of just links to the file, as was expected. If I wanted to change this behavior, I had to interfere with the sphinx latex writer.

So, while the rest of the builders were using the inline toctree directive of that page, the latex builder needed special handling. I am very much looking forward to any generic solutions you have for the latex builder. But, didn't see any solution in the sphinx-external-toc PR, so was enquiring.

[mmcky]

@AakashGfude if we need special handling of the toc for the latex builder then this package is the best place for that.

I had to interfere with the sphinx latex writer.

Do we need to adjust the writer or the translator?
Could we just override the method that is producing output that we don't want (that has the filenames included)?

To assist with this discussion could you please add some screenshots showing the pdf output that sphinx produces. I remember seeing it (and it didn't look good) and that's when we went with the special handling for displaying local contents.

Update: Ah ok I have re-read the thread. @chrisjsewell the issue is that the generic latex writer doesn't render the toc -- it inlines it when building the latex document for inclusion of the documents but doesn't write a representation of the toc in the document. I guess it leans on latex to generate the table of contents. So @AakashGfude addition generates a simple local contents for latex output. I think the best way forward is to use the simple lists that @AakashGfude has put together and we can open an issue to replace it with a more pure latex implementation using some latex package to generate simple local contents listings. @AakashGfude can you open a PR to add in this support.

[chrisjsewell]

I think the best way forward is to use the simple lists that @AakashGfude has put together and we can open an issue to replace it with a more pure latex implementation using some latex package to generate simple local contents listings. @AakashGfude can you open a PR to add in this support.

this should not be that difficult:

1. The point of this PR, and thus any future work, is that for builder specific extensions you can "gather" information in a transform, but you cannot implement any modifications of the AST until a post-transform.
2. The list addition must be optional, and should be one option of how to generate an in-page toc.