Unify HTML animation timings, add CDN export, fix mypy#13
Open
smcolby wants to merge 2 commits into
Open
Conversation
Dashboard / CLI changes: - Slider step frame/transition durations now match the play-button values (700 ms / 300 ms) so scrubbing and playback feel identical. - save_html() gains a use_cdn keyword arg; use_cdn=True emits include_plotlyjs='cdn' for a ~3 MB smaller file suitable for web-hosted viewing (requires internet connection). - CLI (moal simulate) now saves both dashboard_animation.html (self-contained) and dashboard_animation_cdn.html (CDN) by default. - Two new tests: CDN file is smaller and references cdn.plot.ly; slider and play-button timings match in the exported HTML. mypy fixes (0 errors, was 17): - Install pandas-stubs and scipy-stubs (already in dev deps, just missing). - Add rdkit and rdkit.* to [[tool.mypy.overrides]] ignore_missing_imports. - preprocessing.py: wrap Chem.MolToSmiles return in str() to fix no-any-return. - logging_config.py: remove stale type: ignore[attr-defined] on RDLogger call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rdkit from ignore_missing_imports DisableLog is an untyped C++ method in rdkit's stubs (rdkit ships py.typed but does not annotate all C++ extension methods). A targeted type: ignore[attr-defined] on that line is the correct fix. Removed rdkit/rdkit.* from the ignore_missing_imports override since both local and CI environments now use the typed PyPI rdkit wheel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
khuddzu
approved these changes
May 2, 2026
khuddzu
left a comment
khuddzu
left a comment
There was a problem hiding this comment.
A couple of thoughts, but otherwise looks good to me.
There was a problem hiding this comment.
if use_cdn is False what happens here?
Are you wanting to save both everytime?
| logger.warning("SMILES reduced to empty molecule after salt stripping: %s", smiles) | ||
| return None | ||
| return Chem.MolToSmiles(mol, isomericSmiles=True) | ||
| return str(Chem.MolToSmiles(mol, isomericSmiles=True)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Dashboard / CLI
used 350 ms / 220 ms, making scrubbing feel noticeably different from playback.
save_html()gains ause_cdn: bool = Falsekeyword argument. WhenTrue, Plotly JS is loaded from the CDN instead of beingembedded, reducing file size from ~3 MB to ~20 KB (requires internet to view).
moal simulatenow writes bothdashboard_animation.html(self-contained, no internet required) anddashboard_animation_cdn.html(CDN, web-friendly) by default.mypy (0 errors, was 17)
pandas-stubsandscipy-stubswere already listed indevdependencies but not installed — added to the dev install.rdkitandrdkit.*to the[[tool.mypy.overrides]]ignore_missing_importsblock inpyproject.toml(rdkit ships no type stubs).preprocessing.py: wrappedChem.MolToSmilesreturn instr()to satisfyno-any-return.logging_config.py: removed stale# type: ignore[attr-defined]onRDLogger.DisableLog.Tests
Two new tests in
TestSaveHtml:test_save_html_cdn_omits_embedded_plotlyjs— CDN file referencescdn.plot.lyand is substantially smaller than the embedded version.test_slider_and_playback_use_same_frame_duration— verifies 700 ms / 300 ms appear in both the slider step JSON and the injected play script.Notes
cdn.plot.lyendpoint as emitted byplotly.write_html.pandas-stubsandscipy-stubspackages should be installed as part ofpip install -e ".[dev]"going forward.