Opened 2 years ago

Closed 2 years ago

#30315 closed enhancement (fixed)

Switch jsmol to jupyter-jsmol

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.3
Component: packages: standard Keywords: sd111
Cc: kcrisman, novoselt, chapoton, arojas, fbissey, slelievre, nbruin, paulmasson, egourgoulhon, klee, enriqueartal, gh-jcamp0x2a Merged in:
Authors: Matthias Koeppe Reviewers: Joshua Campbell
Report Upstream: N/A Work issues:
Branch: 80a3db1 (Commits, GitHub, GitLab) Commit: 80a3db1582a541ceaaa5098d36d5fedb4c98ee8b
Dependencies: #31002 Stopgaps:

Status badges

Description (last modified by mkoeppe)

Currently, jsmol is installed by the jmol spkg and then symlinked into the jupyter notebook directory.

In this ticket, we switch to https://pypi.org/project/jupyter-jsmol/ (https://github.com/fekad/jupyter-jsmol), which can be installed by standard Python tools.

Discussion: https://groups.google.com/g/sage-devel/c/qKqTmLzHAbg/m/6YxQ_qWUBQAJ

See also:

  • #30316 - Update jmol to 14.31.2, add spkg-configure for jmol
  • #31027 - Make jmol optional

Change History (37)

comment:1 Changed 2 years ago by mkoeppe

Description: modified (diff)

comment:2 Changed 2 years ago by mkoeppe

(deleted)

Last edited 2 years ago by mkoeppe (previous) (diff)

comment:3 Changed 2 years ago by mkoeppe

./sage -pip install jupyter-jsmol installs proper nbextensions:

$ jupyter nbextension list
Known nbextensions:
  config dir: /Users/mkoeppe/s/sage/sage-rebasing/local/etc/jupyter/nbconfig
    notebook section
      jupyter_jsmol/extension  enabled 
      - Validating: OK
      jupyter-js-widgets/extension  enabled 
      - Validating: OK

The installed files can be seen in $SAGE_LOCAL/share/jupyter/nbextensions/jupyter_jsmol/

comment:4 Changed 2 years ago by mkoeppe

Milestone: sage-9.2sage-9.3

comment:5 Changed 2 years ago by mkoeppe

Cc: enriqueartal added

comment:6 Changed 2 years ago by mkoeppe

Branch: u/mkoeppe/switch_jsmol_to_jupyter_jsmol__make_jmol_optional

comment:7 Changed 2 years ago by mkoeppe

Authors: Matthias Koeppe
Cc: gh-jcamp0x2a added
Commit: ac1ab7c844cdeea9346f35e5691755fc713d18c8
Status: newneeds_review

New commits:

6617f87build/pkgs/jupyter_jsmol: New
cbaf661build/pkgs/jmol/spkg-install.in: No longer install jsmol
2ca5328sage.repl.display.jsmol_iframe: Adjust path to jsmol
ac1ab7csage.repl.ipython_kernel.install: No longer symlink jsmol

comment:8 Changed 2 years ago by git

Commit: ac1ab7c844cdeea9346f35e5691755fc713d18c8e6ebaf7e812c5bbd4dfe9d2da0bcee73598d957a

Branch pushed to git repo; I updated commit sha1. New commits:

e6ebaf7build/pkgs/jupyter_jsmol: Add dependencies, install-requires.txt

comment:9 Changed 2 years ago by mkoeppe

sagetex declares jmol as a dependency - what is it used for?

comment:10 in reply to:  9 ; Changed 2 years ago by kcrisman

sagetex declares jmol as a dependency - what is it used for?

I think for generating a 3d image in its documentation, compilation of which is part of its test suite.

comment:11 Changed 2 years ago by mkoeppe

Keywords: sd111 added

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

comment:12 Changed 2 years ago by mkoeppe

Dependencies: #31020

comment:13 Changed 2 years ago by git

Commit: e6ebaf7e812c5bbd4dfe9d2da0bcee73598d957a6a549d97f2051ae6c158adc06ae6b6d86fde24dc

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

21259b2build/pkgs/jupyter_jsmol: New
b5f5f43build/pkgs/jmol/spkg-install.in: No longer install jsmol
a0fe543sage.repl.display.jsmol_iframe: Adjust path to jsmol
6e24bc0sage.repl.ipython_kernel.install: No longer symlink jsmol
52803b4build/pkgs/jupyter_jsmol: Add dependencies, install-requires.txt
7194459build/pkgs/jmol/type: Make it optional
0ed256cbuild/make/Makefile.in: Compute SAGE_CHECK_... earlier, before evaluating dependencies
1f714c1build/pkgs/sagetex/dependencies: Conditionalize order-only deps on SAGE_CHECK_sagetex
6a549d9Merge branch 't/31020/build_make_makefile_in__fix_sage_check_logic__conditionalize_sagetex_dependencies_on_sage_check' into t/30315/switch_jsmol_to_jupyter_jsmol__make_jmol_optional

comment:14 in reply to:  10 Changed 2 years ago by mkoeppe

Replying to kcrisman:

sagetex declares jmol as a dependency - what is it used for?

I think for generating a 3d image in its documentation, compilation of which is part of its test suite.

Thanks! I have adjusted the dependencies in #31020 (now a dependency) so that jmol is only treated as a dependency when SAGE_CHECK is active.

comment:15 Changed 2 years ago by git

Commit: 6a549d97f2051ae6c158adc06ae6b6d86fde24dcae970c2c80a24bfeab28a1748881147c6cd12bf8

Branch pushed to git repo; I updated commit sha1. New commits:

3a45d72build/pkgs/jupyter_jsmol/spkg-install.in: Do not rebuild the javascript file
a8e0364Specify bdist temp folder for WSL compatibility
a3077c7Use mktemp
c5a14a3build/bin/sage-dist-helpers (sdh_setup_bdist_wheel): New
fb10429build/pkgs/*/spkg-install.in: Use new function sdh_setup_bdist_wheel
32bf463Merge branch 't/31002/public/build/bdist_wsl' into t/30315/switch_jsmol_to_jupyter_jsmol__make_jmol_optional
ae970c2build/pkgs/jupyter_jsmol/spkg-install.in: Use new function sdh_setup_bdist_wheel

comment:16 Changed 2 years ago by mkoeppe

Dependencies: #31020#31002, #31020

comment:17 Changed 2 years ago by mkoeppe

Description: modified (diff)

comment:18 Changed 2 years ago by kcrisman

That is not a good idea. Because we want people to be able to build the documentation for SageTeX, which after all isn't "really" upstream. Instead, we should see if jsmol or some other tool suffices to build 3d pictures in SageTeX, which is a very useful use case for it. Untested = broken, and I don't think that we've ever had a dependency only in SAGE_CHECK before.

comment:19 in reply to:  18 ; Changed 2 years ago by kcrisman

and I don't think that we've ever had a dependency only in SAGE_CHECK before.

Though my info on the build system is pretty outdated :-)

comment:20 in reply to:  19 Changed 2 years ago by mkoeppe

Replying to kcrisman:

and I don't think that we've ever had a dependency only in SAGE_CHECK before.

Though my info on the build system is pretty outdated :-)

Actually, we've introduced a few of these this year because the complexity of Python test frameworks has escalated and we don't want to ship all of their dependencies. Currently, networkx, rpy2, and sage_sws2rst all have check-only dependencies.

comment:21 Changed 2 years ago by kcrisman

Interesting. I wonder whether jsmol might be a drop-in replacement for this specific use case though. Or even Tachyon ...

comment:22 in reply to:  21 Changed 2 years ago by novoselt

Replying to kcrisman:

Interesting. I wonder whether jsmol might be a drop-in replacement for this specific use case though. Or even Tachyon ...

What's the status of generating 3D plots and saving them using threejs? I think for SageTeX it is irrelevant what is producing pictures, just show that it works well. Tachyon is not the best in this regard.

comment:24 Changed 2 years ago by mkoeppe

Description: modified (diff)
Summary: Switch jsmol to jupyter-jsmol, make jmol optionalSwitch jsmol to jupyter-jsmol

comment:25 Changed 2 years ago by git

Commit: ae970c2c80a24bfeab28a1748881147c6cd12bf8bf3c1dc8956ce14720b0a230ab7d99198de64c40

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e57f927Merge branch 't/31002/public/build/bdist_wsl' into t/30315/switch_jsmol_to_jupyter_jsmol__make_jmol_optional
017b816build/pkgs/jupyter_jsmol: New
db1b38cbuild/pkgs/jmol/spkg-install.in: No longer install jsmol
95a7131sage.repl.display.jsmol_iframe: Adjust path to jsmol
98b83f5sage.repl.ipython_kernel.install: No longer symlink jsmol
40a8823build/pkgs/jupyter_jsmol: Add dependencies, install-requires.txt
bf3c1dcbuild/pkgs/jupyter_jsmol/spkg-install.in: Do not rebuild the javascript file

comment:26 Changed 2 years ago by mkoeppe

Dependencies: #31002, #31020#31002

I have narrowed this ticket to only separating the jmol and jsmol installation.

comment:27 Changed 2 years ago by mkoeppe

Making jmol optional, and the discussion about new technologies to create images from 3d graphics, is now on #31027.

comment:28 Changed 2 years ago by gh-jcamp0x2a

I was able to verify that JSmol still works correctly in both Jupyter and JupyterLab. However, I did notice a test failure in src/sage/repl/ipython_kernel/install.py due to the new deprecation warning:

File "src/sage/repl/ipython_kernel/install.py", line 142, in sage.repl.ipython_kernel.install.SageKernelSpec.use_local_jsmol
Failed example:
    spec.use_local_jsmol()
Expected nothing
Got:
    doctest:warning
      File "/home/joshua/sage/src/bin/sage-runtests", line 182, in <module>
        err = DC.run()
    [...]
    :
    DeprecationWarning: Symlinking jsmol is no longer necessary
    See https://trac.sagemath.org/30315 for details.

Also, after removing the local/share/jsmol folder since it is no longer needed (and wouldn't be present on a clean install), I get a couple more test failures in the same file:

File "src/sage/repl/ipython_kernel/install.py", line 144, in sage.repl.ipython_kernel.install.SageKernelSpec.use_local_jsmol
Failed example:
    os.path.isdir(jsmol)
Expected:
    True
Got:
    False

and

File "src/sage/repl/ipython_kernel/install.py", line 146, in sage.repl.ipython_kernel.install.SageKernelSpec.use_local_jsmol
Failed example:
    os.path.isfile(os.path.join(jsmol, "JSmol.min.js"))
Expected:
    True
Got:
    False

comment:29 Changed 2 years ago by git

Commit: bf3c1dc8956ce14720b0a230ab7d99198de64c407732e2eac50a3e2c0f865304ce4eecd26c6ff960

Branch pushed to git repo; I updated commit sha1. New commits:

f2a02d9Get rid of JSMOL_DIR
7732e2ebuild/pkgs/jmol/SPKG.rst: Update

comment:30 Changed 2 years ago by mkoeppe

Thanks. This function, after all, does not make sense any more, so there's no point in trying to go through deprecation. I have now removed it completely (alongside with the variable JSMOL_DIR).

comment:31 Changed 2 years ago by gh-jcamp0x2a

Reviewers: Joshua Campbell
Status: needs_reviewpositive_review

Looks good. JSmol still works in Jupyter and JupyterLab.

comment:32 Changed 2 years ago by mkoeppe

Thank you!

comment:33 Changed 2 years ago by git

Commit: 7732e2eac50a3e2c0f865304ce4eecd26c6ff96080a3db1582a541ceaaa5098d36d5fedb4c98ee8b
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

80a3db1build/pkgs/jupyter_jsmol/spkg-install.in: Use sdh_setup_bdist_wheel

comment:34 Changed 2 years ago by mkoeppe

Forgot to actually use #31002 - this change is necessary for correct installation on WSL

comment:35 in reply to:  34 Changed 2 years ago by gh-jcamp0x2a

Status: needs_reviewpositive_review

Replying to mkoeppe:

Forgot to actually use #31002 - this change is necessary for correct installation on WSL

Ok. I tested this on Ubuntu WSL earlier without any installation issues. If I understand #31002 correctly, the problem was occurring due to mounting Windows folders, which I'm not doing, so that's probably why I didn't run into it.

I've gone ahead and re-verified that JSmol still works in Jupyter and JupyterLab.

comment:36 Changed 2 years ago by mkoeppe

Thanks again!

comment:37 Changed 2 years ago by vbraun

Branch: u/mkoeppe/switch_jsmol_to_jupyter_jsmol__make_jmol_optional80a3db1582a541ceaaa5098d36d5fedb4c98ee8b
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.