Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements for Families Module (interactive docs, better io, typo fixes) #252

Open
wants to merge 59 commits into
base: master
Choose a base branch
from

Conversation

josephburkhart
Copy link
Contributor

@josephburkhart josephburkhart commented Feb 6, 2025

Description

I have made several changes related to the families module:

1. Interactive Documentation

I have created interactive documentation for each of the shape families. Previously, users had to go digging through the SI of cited papers in order to know what ID and name they needed to get a specific shape. Now, there is a new section of the documentation, accessible via the sidebar, titled "Shape Family Tables". Tables contain basic information about the shapes (ID, name, number of vertices, number of faces), as well as interactive models that allow users to actually see what the shapes look like.

To make these new pages, I created a minimal sphinx extension, some CSV data tables, and exported X3D models for all of the members of the discrete shape families. I also modified the default page template and created some custom CSS.

Two things I need feedback on:

  • Are the table pages sufficiently performant on the client side? All of the tables work perfect on my machine except for the Johnson Solids page. On my machine, the Johnson Solids page takes a little while to load, but once it is loaded the interactivity is still decently smooth. Please let me know if this is different for you.
  • Do we need/want pages for the pyramid/dipyramids and the prisms/antiprisms? I couldn't tell if we would want that since PyramidDipyramidFamily and PrismAntiprismFamily are deprecated, so I went ahead and made their pages anyway.

2. Better IO to X3D (and by extension to HTML)

In order to make the interactive models more clear, I modified io.to_x3d() to highlight the shape edges. Since io.to_html() calls to_x3d() under the hood, it was also affected. I created new control files for the io unit tests to reflect the updated functionality. In the process, I also tweaked test_io.py so that it generated new control files better.

3. Typo fixes in JSON files

During the development process, I encountered several typos in the reference geometry data stored in JSON files in families/data. These typos are as follows:

  • science1220869.json: for shape J86, the name was given as "Spenocorona", but it should be "Sphenocorona"
  • _previous_science1220869.json: same as previous
  • johnson.json: for shape "Sphenocorona" (note the spelling is already correct here), the vertices did not match those in science1220869.json, so I updated the vertices in this file to match those in that file.

That last typo is concerning. I only found it because when I corrected the spelling of "Sphenocorona" in the first two files, I stated getting an AssertionError in tests/test_shape_families.py::test_science_family() for name="J86". If you look at the code for that test, KeyErrors for shapes in the Johnson Family cause a short-circuit in the check for vertices equality. As far as I can tell, there are no more misspellings in shape names, but there may still be name mismatches that I haven't found yet. Therefore, I strongly suggest we check every one of the names in johnson.json to make sure this problem doesn't happen again.

Personally, I'm not sure I really understand why we need test_science_family(). It seems to me that its only function is to check that shapes instantiated through data in individual family JSON files (platonic.json, archimedean.json, etc.) match those have the same vertices as shapes instantiated through data from the Damasceno paper (science1220869.json), and since those JSON files should not ever change (except clearly here), I don't see why we should test that equivalency more than once. Perhaps I'm missing something?

4. Build changes

The extension I wrote for item 1 above required me to make some small changes to the makefile and readthedocs build configuration. While the source X3D files should be copied to the build directory automatically, files in source/_static are only copied to the build directory after extensions are parsed, which causes Sphinx to throw a FileNotFoundError as soon as it encounters the custom role defined in the extension. As far as I can tell, this issue is documented as expected behavior in sphinx-doc/sphinx#2090 and sphinx-doc/sphinx#1810. For this reason, I have modified the makefile to copy all X3D files to the build directory before running sphinx-build. I have also updated .readthedocs.yml accordingly.

Finally, I have also updated the docs' environment.yml file so that the notebooks build without errors.

Motivation and Context

These changes make the documentation for the families module more accessible and correct several bugs in the internal data and testing apparatus.

Types of Changes

  • Documentation update
  • Bug fix
  • New feature
  • Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

josephburkhart and others added 30 commits February 3, 2025 13:39
This commit resolves an issue where Sphinx will not copy static X3D files from the source to the build directory before loading extensions, which results in a FileNotFoundError and causes the build process to exit. It is documented in Sphinx issues #2090 and #1810 that this is expected behavior.
@joaander
Copy link
Member

@joaander
Copy link
Member

Or perhaps you only need to explicitly list the files in the static path?
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html_static_path

@josephburkhart
Copy link
Contributor Author

josephburkhart commented Feb 10, 2025

I just now saw your suggestions. I tried using html_extra_path, but I wasn't able to get it to work - the issue was that the copying was happening late enough in the build procedure that I was still getting FileNotFoundError. I can double-check again on my local machine.

Edit: confirmed. When I put html_extra_path = ["_static/x3d"] in conf.py, the build fails with a FileNotFoundError, because the files have not yet been copied when the extension is parsed. As far as I can tell, this issue is documented as expected behavior in sphinx-doc/sphinx#2090 and sphinx-doc/sphinx#1810.

@joaander
Copy link
Member

Ah, that is unfortunate.

@joaander
Copy link
Member

The Johnson solids page takes about ~5 seconds to load on my M1 Mac.

@DomFijan
Copy link
Contributor

I can see all the shapes (and interact with them) on Platonic solids and Pyramids/Bipyramids. On other pages I can either see no shapes, or only some shapes. I tried dark/light mode and two browsers: Edge and chrome, both showing exactly same issues (OS: Windows). Loading times are good.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants