Fix of Sphinx Gallery display of Chemiscope Visualizer #334
Conversation
This will allow to test the sphinx-gallery integration without leaving this repo
docs/_static/js/chemiscope.min.js
Outdated
There was a problem hiding this comment.
This file should not be committed to the repo long term (since it is pretty big and auto-generated). It's fine for now, I'll help replace it with the generated version of the file later
There was a problem hiding this comment.
I see, thanks!
docs/src/conf.py
Outdated
| import sphinx_bootstrap_theme | ||
|
|
||
| sys.path.insert(0, os.path.abspath('.')) | ||
| from scraper import SphinxGalleryScraper |
There was a problem hiding this comment.
this should something like from chemiscope.sphinx_gallery import ChemiscopeScraper, i.e. the code should live inside the chemiscope package to be usable by anyone with chemiscope installed (We should not just use it in this repo)
| sphinx_gallery_conf = { | ||
| "examples_dirs": os.path.join(ROOT, "python", "examples"), | ||
| "gallery_dirs": "examples", | ||
| "filename_pattern": ".*", |
There was a problem hiding this comment.
I guess this is required to include the JSON files?
docs/src/scraper.py
Outdated
| <!-- Font-awesome --> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/5.11.2/css/all.min.css" /> | ||
| <link rel="icon" type="image/png" href="../../../_static/chemiscope-icon.png" sizes="32x32" /> |
There was a problem hiding this comment.
I don't think these are needed here, since we are not adding any icons
There was a problem hiding this comment.
There is an icon for loading
There was a problem hiding this comment.
hmm, might be worth to try to use something smaller than the whole of fa here. Maybe a small svg instead?
docs/src/scraper.py
Outdated
| ></script> | ||
|
|
||
| <!-- JS scripts --> | ||
| <script src="../../../_static/js/chemiscope.min.js"></script> |
There was a problem hiding this comment.
You should load these from gallery_dirs, not the root!
docs/src/scraper.py
Outdated
| self.copy_chemiscope_files(target_dir, source_files) | ||
|
|
||
| # Use the custom html content | ||
| widget._repr_html_ = lambda: self.generate_html_content(widget.value) |
There was a problem hiding this comment.
Instead of passing widget.value here, you should save the corresponding data in a JSON (or JSON/gzip file) — maybe using sphinx-gallery to create a filename like fig_xxx_.json.gz, there should be an iterator (block_vars["image_path_iterator"]) to generate such names — and then load it from JS with fetch.
docs/src/scraper.py
Outdated
|
|
||
| def __call__(self, block, block_vars, gallery_conf): | ||
| variables = block_vars.get("example_globals", {}) | ||
| widget = variables.get("___") |
There was a problem hiding this comment.
this might fail if the user tries to show multiple chemiscope in a single sphinx-gallery "cell"
docs/src/scraper.py
Outdated
| widget._repr_html_ = lambda: self.generate_html_content(widget.value) | ||
|
|
||
| # Use sphinx-gallery standard scraper | ||
| return matplotlib_scraper(block, block_vars, gallery_conf) |
There was a problem hiding this comment.
do we need to do this? since the matplotlib scrapper is already in the list of scrappers in the conf
…es + temprorary fix ase version
|
|
Works great and I think this only needs some polish before we can merge and release. Amazing work @sofiia-chorna I'd say this is worthy of a point release at the very least, @Luthaf and I will also help starting on Tuesday. Meanwhile, something that clearly needs some polish is the documentation of this new feature. If I understand correctly, this can now be used in plain sphinx too - which is fantastic - but there's no example or explanation of how. @sofiia-chorna could you perhaps write down some docs? I'd rename the "use in sphinx-gallery" to a more generic "use with spinx" and discuss both direct inclusion and the generation from a python file using chemiscope.show. |
|
OK, it works - and so does the software cookbook so all is really nice, great job @sofiia-chorna . in it, however this does not work. Could you add a documentation of how this is supposed to be used wit (and without) sphinx-gallery? |
|
closing in favor of #341 |
Allow to display chemiscope viewers in sphinx documentations using sphinx-gallery.