search

jupyterlite / jupyterlite-sphinx

Sphinx extension using JupyterLite to render Notebooks
https://jupyterlite-sphinx.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
68 stars 21 forks source link

Refactor source code and split it up into multiple files? #162

Open agriyakhetarpal opened 8 months ago

agriyakhetarpal commented 8 months ago

Problem

An issue I found while contributing to the Sphinx extension, coupled with the lack of a CONTRIBUTING.md document (which is another, unrelated issue) is that it was a bit difficult to navigate what was going on for someone whose Sphinx skills have been a bit rusty since quite some time. I would like to put in a request to refactor the codebase and move things around, and here are my suggestions:

Proposed Solution

  1. A complete refactor might take a lot of time and hinder development efforts, but at least moving the IFrame classes to a non-namespace sub-module jupyterlite_sphinx/iframes/ and then similarly moving the directives to jupyterlite_sphinx/directives/ can work
  2. Another level of hierarchy exists with the Directive classes, where all of them are subclasses that derive from _LiteDirective, which itself derives from the standard SphinxDirective. These would be likely candidates to split up into multiple files, and it should be easy to fix up their imports back into jupyterlite_sphinx.py (it might make sense to rename the file because of the folder structure, i.e., jupyterlite_sphinx.main sounds a bit better than jupyterlite_sphinx.jupyterlite_sphinx)
  3. Besides that, I'm not sure if anything else can be extracted out of a refactoring process (the jupyterlite_sphinx.js and jupyterlite_sphinx.css files are standalone in their usage and exist only for copying into the _static folder for a documentation website.

Additional context

Though it is a bit outdated and unmaintained at the time of writing, another project: https://github.com/sphinx-contrib/cookiecutter has a nice structure to offer. However, it may or may not be needed here for now because the packaging is modern with hatch anyway and a test suite does not exist (and that may not be required anytime soon?).

cc: @steppi

steppi commented 8 months ago

cc: @steppi

Thanks @agriyakhetarpal, but I don't really have an opinion on this one way or the other, beyond that I agree it would be good to add a CONTRIBUTING.md. I had no experience working on sphinx extensions prior to this and just fit the try_examples directive into the existing project structure. If you want to work on this, I think this should be decided by the other maintainers.

martinRenou commented 8 months ago

Hey! No strong feeling about whether or not to split the package in multiple files. Though could be good indeed if we continue adding features to it.

agriyakhetarpal commented 8 months ago

Sure! My suggestion here is quite trivial, really, so I'll be happy to start my work on this once some of the recent PRs can get merged – and maybe after we come out with a release with those, to not cause any trouble(s) before a release.

martinRenou commented 8 months ago

Sounds great!