spatialaudio / nbsphinx

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

Split code into several files? #667

Open mgeier opened 2 years ago

mgeier commented 2 years ago

I just stumbled over this behavior in an up-to-date version. I tried to understand what is going on in https://github.com/spatialaudio/nbsphinx/blob/master/src/nbsphinx.py but reading and comprehending close to 2400 lines of code is no joke. @mgeier is there a chance that in the future this code is split up into several files, each dealing with specific concerns? That would make it much easier to contribute.

Originally posted by @1kastner in https://github.com/spatialaudio/nbsphinx/issues/315#issuecomment-1220819298

1kastner commented 2 years ago

Thanks for creating this issue based on my lengthy comment.

1kastner commented 2 years ago

One idea that came to my mind was - is it possible to fragment the code according to the phases as listed at https://www.sphinx-doc.org/en/master/extdev/index.html#build-phases? That'd be really handy for issues like the TOC question or when it comes to link resolution. Another idea could be to extract the template code into separate text files.

mgeier commented 1 year ago

One idea that came to my mind was - is it possible to fragment the code according to the phases as listed at https://www.sphinx-doc.org/en/master/extdev/index.html#build-phases? That'd be really handy for issues like the TOC question or when it comes to link resolution.

I'm not so sure. I guess only two of the phases would be relevant, phase 1 and phase 3, right?

So those would be submodules? Something like nbsphinx.reading and nbsphinx.resolving?

Another idea could be to extract the template code into separate text files.

Yeah, on the one hand this would definitely be an option, on the other hand the template code is a work-around anyway (see #36).

It would be nice to move the LaTeX and CSS stuff into separate files, but it never bothered me enough to look into it.

In case of CSS I'm also not so sure about the performance implications of having the CSS as separately hosted file vs. inline in the HTML.

1kastner commented 1 year ago

Your propositions sound great to me! As #36 has been around for a while, the interims solution seems to be good enough to stick with it for another few months or years? Well, I guess that the CSS could remain embedded into the HTML file as long as the HTML file is extracted from the Python code.

I could try to extract LaTeX and HTML/CSS into separate files and create a related PR if you'd like.

mgeier commented 1 year ago

Yes, #36 might stay open for a while ... we could try to move the RST template to a separate file until then.

Well, I guess that the CSS could remain embedded into the HTML file as long as the HTML file is extracted from the Python code.

I don't understand what you mean by that.

I could try to extract LaTeX and HTML/CSS into separate files and create a related PR if you'd like.

Yes, sure, if you want to. You might have noticed that this is not very important to me. But if it's important to you, we can try to tackle it!

However, please note that CSS_STRING contains a few places where nbsphinx_responsive_width and nbsphinx_prompt_width has to be inserted.

mgeier commented 1 year ago

I've started with this in #705.

I need this for the new gallery style (to store some default images).

For now, I'm only planning to move the CSS out of the main file. If you want to move more things, feel free to create a PR.

mgeier commented 1 year ago

It turns out that having the LaTeX stuff in a separate files makes it easier to include a nbsphinx-based project into another LaTeX file, so I created #708 which moves all LaTeX code into a separate file nbsphinx.sty.

Now the RST_TEMPLATE still remains in the main Python file. Again, I'm not planning to move that for now.

1kastner commented 1 year ago

Thank you @mgeier for pushing the idea forward. I am currently involved in a quite time-consuming project. I hope to give a helping hand later!