Opened 2 years ago
Closed 20 months ago
#30972 closed enhancement (fixed)
Versioned installation of threejs
Reported by: | paulmasson | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-9.4 |
Component: | graphics | Keywords: | threejs, jsmol, jupyter, sd111 |
Cc: | gh-jcamp0x2a, fbissey, mkoeppe, charpent, enriqueartal, jhpalmieri, slabbe, arojas, defeo, novoselt, dimpase | Merged in: | |
Authors: | Matthias Koeppe, Joshua Campbell | Reviewers: | Joshua Campbell, Matthias Koeppe, Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 935dcef (Commits, GitHub, GitLab) | Commit: | 935dcef836f84473e3c3368b0999b4ec310c0eb3 |
Dependencies: | Stopgaps: |
Description (last modified by )
From Matthias Koeppe on #30915:
When we start using Sage with a system jupyter notebook (Meta-ticket #30306), there is an issue involving the installation directory share/jupyter/nbextensions/threejs
, and the fact that there will be several unrelated (and possibly mutually incompatible) copies of that.
Here is an example.
Let's say:
- system jupyter is installed in
/usr
, so the notebook server is accessing/usr/share/jupyter/nbextensions
. - Sage-9.x may be installed in
/home/x/sage-9.x/local/
and may need threejs r122. - Sage-9.y may be installed in
/home/x/sage-9.y/local/
and may need threejs r155. - Let's say r122 and r155 are mutually incompatible.
As @enriqueartal has reported in #30915, our offline threejs graphics needs /usr/share/jupyter/nbextensions/threejs/
. But the system does not provide it -- it is only provided by Sage (and, after #30915 is the result of a custom build (fork) of threejs).
If we create a symlink /usr/share/jupyter/nbextensions/threejs
-> /home/x/sage-9.x/local/share/jupyter/nbextensions/threejs
, then Sage 9.x will work with the system jupyter notebook; but Sage 9.y will not.
This means that we need a versioned installation scheme so that offline threejs graphics can access the specific version it needs even if the user is accessing Sage through the system notebook.
In this ticket we also rename directories from threejs
to threejs-sage
(in share/
and share/jupyter/nbextensions/
) to reflect the fact that it is a Sage-specific fork. This will reduce the need for downstream patching.
(On this ticket we ignore the separate issue that installing such symlinks is not user-friendly -- see #30123 Repackage Sage's cropped threejs
as a pip-installable package jupyter-threejs-sage
for a follow-up.)
Change History (56)
comment:1 Changed 2 years ago by
comment:2 follow-up: 3 Changed 2 years ago by
That's not a solution because a user may want to use two different Sage versions at the same time.
comment:3 follow-up: 5 Changed 2 years ago by
Replying to mkoeppe:
That's not a solution because a user may want to use two different Sage versions at the same time.
Why exactly would someone want to do this? I can see a user not wanting to upgrade to the newest version, but why would they use an older version when the newer one will almost certainly be an improvement?
comment:4 follow-up: 7 Changed 2 years ago by
Should we use JUPYTER_PATH for this kind of things? https://jupyter.readthedocs.io/en/latest/use/jupyter-directories.html
comment:5 follow-up: 6 Changed 2 years ago by
Replying to paulmasson:
Replying to mkoeppe:
That's not a solution because a user may want to use two different Sage versions at the same time.
Why exactly would someone want to do this?
Users have complex needs.
comment:6 follow-up: 49 Changed 2 years ago by
Replying to mkoeppe:
Replying to paulmasson:
Replying to mkoeppe:
That's not a solution because a user may want to use two different Sage versions at the same time.
Why exactly would someone want to do this?
Users have complex needs.
Considering that I don't even use Sage, I'll need a better answer than this before devoting more time to this issue. We are not required to support every feature desired by end users. We can simply tell them to run only one kernel at a time. Who is making the decision to support this option?
comment:7 Changed 2 years ago by
Replying to fbissey:
Should we use JUPYTER_PATH for this kind of things? https://jupyter.readthedocs.io/en/latest/use/jupyter-directories.html
I don't think this can be solved by using environment variables. The issue is that the notebook may access different kernels when the kernelspec is installed, and (apparently) the notebook webserver is expected to serve the local threejs files. Now these files are kernel-specific (because they are tied to the sagelib version).
Francois, how do you currently install these files in gentoo packaging? (Overall I think we need some input from downstream packaging to make progress here.)
comment:8 Changed 2 years ago by
Description: | modified (diff) |
---|---|
Summary: | Symlinks and system packages → Versioned installation of threejs |
comment:9 Changed 2 years ago by
For "sagelib" I disable https://github.com/sagemath/sage/blob/develop/src/sage/repl/ipython_kernel/install.py#L281 because of the way I have to install documentation following Gentoo's guidelines (not everyone in Gentoo is happy with the current way of doing this) and I do the stuff from that line manually. For the rest, sagelib's kernel install is used - it wasn't always the case.
I just change the kernel specs to use the current python directly.
I don't support any versioning of sage whatsoever. Multiple pythons are supported and work thanks to the current Gentoo python infrastructure that supports it. But stuff in /usr/share/jupyter/
has to be somewhat python independent really.
fbissey@moonloop ~ $ ll /usr/share/jupyter/kernels/sagemath/ total 20K drwxr-xr-x 2 root root 4.0K Nov 27 12:55 . drwxr-xr-x 4 root root 4.0K Apr 21 2020 .. lrwxrwxrwx 1 root root 30 Nov 27 12:53 doc -> ../../../doc/sage-9999/html/en -rw-r--r-- 1 root root 134 Nov 27 12:53 kernel.json lrwxrwxrwx 1 root root 93 Nov 27 12:53 logo-64x64.png -> ../../../../..//usr/lib/python3.8/site-packages/sage/ext_data/notebook-ipython/logo-64x64.png lrwxrwxrwx 1 root root 87 Nov 27 12:53 logo.svg -> ../../../../..//usr/lib/python3.8/site-packages/sage/ext_data/notebook-ipython/logo.svg fbissey@moonloop ~ $ ll /usr/share/jupyter/nbextensions/ total 12K drwxr-xr-x 3 root root 4.0K Nov 27 12:55 . drwxr-xr-x 5 root root 4.0K Oct 26 21:37 .. lrwxrwxrwx 1 root root 16 Nov 27 12:53 jsmol -> /usr/share/jsmol drwxr-xr-x 2 root root 4.0K Nov 19 15:56 jupyter-js-widgets lrwxrwxrwx 1 root root 18 Nov 27 12:53 mathjax -> /usr/share/mathjax lrwxrwxrwx 1 root root 23 Nov 27 12:53 threejs -> /usr/share/sage/threejs
sage-9999
is just sage straight from the git develop branch, so right now 9.2.beta3 (but in my case, Volker's develop branch).
Users who want versioning, usually are after reproducibility. In that case system packaging is not part of the solution. containers on the other are an ideal tool for that.
comment:10 Changed 2 years ago by
Dependencies: | → #30915 |
---|
comment:11 Changed 2 years ago by
I would propose the following installation scheme in share/
and (via symlink) share/jupyter/nbextensions/
:
- threejs-sage (renamed to indicate that it is a fork) - r122/ (subdirectory for the version, replaces meaningless "build") - three.min.js
comment:12 Changed 2 years ago by
I would also propose to store the version of threejs
that is required by the scripts / templates in src/sage/ext_data/threejs/
in a text file in that directory.
From a packaging point of view it makes no sense to read this from the version
file that is installed in THREEJS_DIR
.
comment:13 Changed 2 years ago by
Branch: | → u/mkoeppe/versioned_installation_of_threejs |
---|
comment:14 Changed 2 years ago by
Commit: | → 938bff40dbf6a3d5c96b5a90cffff6691c26db66 |
---|
Here's a draft (untested)
Last 10 new commits:
63530d2 | Update script link generation
|
82b8888 | Update online link
|
55449d8 | Update upstream URL
|
eed14d3 | Fix doctest and rename more 'scripts' to 'script'
|
c097439 | Revert threejs_scripts method name changes
|
c2204a4 | Merge branch 'u/gh-jcamp0x2a/threejs-r122' of git://trac.sagemath.org/sage into t/30972/versioned_installation_of_threejs
|
1fc3d4c | build/pkgs/threejs: Install in /threejs-sage/rVERSION/
|
c8e707c | src/sage/repl/ipython_kernel/install.py: Change from threejs to threejs-sage
|
5e1ca4e | src/sage/repl/ipython_kernel/install.py, src/sage/env.py, build/pkgs/sage_conf: Change from threejs to threejs-sage
|
938bff4 | Read required threejs version from ext_data
|
comment:15 Changed 2 years ago by
Commit: | 938bff40dbf6a3d5c96b5a90cffff6691c26db66 → f847738d8367ea931af8f955a72cf1a4f64913ab |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
f847738 | src/sage/repl/rich_output/backend_ipython.py: Another change threejs -> threejs-sage
|
comment:16 Changed 2 years ago by
Authors: | → Matthias Koeppe |
---|
comment:17 Changed 2 years ago by
Description: | modified (diff) |
---|
comment:18 Changed 2 years ago by
Status: | new → needs_review |
---|
comment:19 Changed 2 years ago by
Description: | modified (diff) |
---|
comment:20 Changed 2 years ago by
I guess I can accomodate that. I already ship threejs as a sage specific component in /usr/share/sage
. I have to think if I will allow several versions of threejs on the system at once.
comment:21 Changed 2 years ago by
Commit: | f847738d8367ea931af8f955a72cf1a4f64913ab → d13177d1a89cc6bb643411b8bb8b233f9e1a54a6 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
d13177d | src/sage/repl/rich_output/display_manager.py: Fix up imports
|
comment:22 follow-up: 23 Changed 2 years ago by
This version passes the testsuite. Manual testing would be appreciated.
comment:23 Changed 2 years ago by
Authors: | Matthias Koeppe → Matthias Koeppe, Joshua Campbell |
---|---|
Branch: | u/mkoeppe/versioned_installation_of_threejs → u/gh-jcamp0x2a/versioned_installation_of_threejs |
Commit: | d13177d1a89cc6bb643411b8bb8b233f9e1a54a6 → 47e795ba807db4eb4c742099f3d56cada76fc9b2 |
Replying to mkoeppe:
This version passes the testsuite. Manual testing would be appreciated.
I had to update the path in sage.repl.rich_output.backend_ipython.BackendIPythonNotebook.threejs_offline_scripts
similar to what you did for BackendIPythonCommandline
in order to avoid 404s in jupyter/jupyterlab.
Since I already had the fix locally, I hope you don't mind that I've gone ahead and pushed it and pointed the ticket at it.
Ran both online=True
and online=False
for the command-line, jupyter, and jupyterlab. I also verified that the CDN fallback works for jupyter and jupyterlab (for the "Save as HTML" feature).
New commits:
47e795b | Fix 404 fetching three.min.js from Jupyter/JupyterLab
|
comment:25 follow-ups: 26 27 29 Changed 2 years ago by
If the plan is to start shipping multiple old versions of Three.js with every new Sage release, then I don't much like that idea. If goes against the entire sense of improving software with a new release.
I would rather implement an idea Joshua raised some time ago on a ticket I can't locate: separate the Three.js viewer as a stand-alone package and keep all the versioning issues there. All of the dependencies of Sage on Three.js are in the template. If that were a separate package it would track the correct version of Three.js by default.
Joshua, can you remember which ticket had your thoughts on this? Are you game to take a crack at implementing this?
comment:26 Changed 2 years ago by
Replying to paulmasson:
[...] goes against the entire sense of improving software with a new release.
Hardly. It's just like installing old versions of shared libraries in a system in addition to the current libraries so that old programs can continue to run.
comment:27 Changed 2 years ago by
Replying to paulmasson:
separate the Three.js viewer as a stand-alone package and keep all the versioning issues there. All of the dependencies of Sage on Three.js are in the template.
The problem remains that the Javascript code needs to be accessed by the Jupyter installation (notebook server), whereas the template needs to be accessed by Sage (Python code). These are in general separate installations - as I have explained with the example in the ticket description. So I don't think that this will help unless I misunderstand something there.
comment:28 Changed 2 years ago by
Also, note that this ticket here does not change how many versions of the javascript file are installed by the distribution. It only changes the installation path to include a version number, which solves the problem described in the ticket description.
comment:29 Changed 2 years ago by
Replying to paulmasson:
I would rather implement an idea Joshua raised some time ago on a ticket I can't locate: separate the Three.js viewer as a stand-alone package and keep all the versioning issues there. All of the dependencies of Sage on Three.js are in the template. If that were a separate package it would track the correct version of Three.js by default.
Joshua, can you remember which ticket had your thoughts on this?
I believe this was comment:4:ticket:30620 and comment:7:ticket:30620 ("Three.js: future of examples/js files").
I don't think it'd solve the present versioning issue, though. We'd just be worrying about providing multiple offline versions of this new package instead.
comment:30 Changed 2 years ago by
Cc: | jhpalmieri slabbe added |
---|---|
Priority: | major → critical |
comment:31 Changed 2 years ago by
Cc: | arojas added |
---|
comment:32 Changed 2 years ago by
Keywords: | sd111 added |
---|
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111
comment:33 Changed 2 years ago by
Cc: | defeo added |
---|
comment:34 Changed 2 years ago by
Cc: | novoselt added |
---|
comment:35 Changed 2 years ago by
Reviewers: | → Joshua Campbell |
---|---|
Status: | needs_review → positive_review |
I went ahead and re-tested the branch in the command-line, jupyter, and jupyterlab with both online=True
and online=False
and checked that plots saved using the Save as HTML
feature still function.
I don't know how frowned upon it is to positively review a ticket you're listed as one of the authors of, but since this ticket has been in review for awhile without a bite, I'm going to go ahead and do so. Please feel free to change it back if there is any objection.
comment:36 Changed 2 years ago by
Reviewers: | Joshua Campbell → Joshua Campbell, Matthias Koeppe |
---|
Thanks. I have reviewed the changes that you made on the ticket.
comment:37 follow-up: 38 Changed 2 years ago by
Status: | positive_review → needs_info |
---|
I strongly object to the two authors reviewing this ticket. I have yet to receive a cogent answer as to why someone would want to run two or more versions of Sage at the same time. This ticket also needs more input from distribution managers, who may not want to support this behavior. There need to be other voices clearly in favor of the direction this ticket moves, a direction I for one do not support.
comment:38 Changed 2 years ago by
Replying to paulmasson:
I strongly object to the two authors reviewing this ticket. I have yet to receive a cogent answer as to why someone would want to run two or more versions of Sage at the same time. This ticket also needs more input from distribution managers, who may not want to support this behavior. There need to be other voices clearly in favor of the direction this ticket moves, a direction I for one do not support.
Apologies. I did not realize there were outstanding questions about the use case's validity, of which I can not personally attest. Just saw that it was in review for awhile yet functioning 'correctly' (to ticket's spec).
comment:39 Changed 2 years ago by
Paul, if you have a specific technical objection to the change that this ticket proposes, it would probably be helpful for the discussion to try to articulate it.
comment:40 Changed 2 years ago by
Let's discuss what the ticket changes step by step.
1) renames directories from threejs
to threejs-sage
(in share/ and share/jupyter/nbextensions/) to reflect the fact that it is a Sage-specific fork
for example in this change
-THREEJS_DIR = "@prefix@/share/threejs" +THREEJS_DIR = "@prefix@/share/threejs-sage"
Any concerns or objections to that?
comment:41 Changed 2 years ago by
Status: | needs_info → needs_review |
---|
comment:42 Changed 2 years ago by
Branch: | u/gh-jcamp0x2a/versioned_installation_of_threejs → u/mkoeppe/versioned_installation_of_threejs |
---|
comment:43 Changed 2 years ago by
Commit: | 47e795ba807db4eb4c742099f3d56cada76fc9b2 → 0b24127485224cd1f4d16f5200132cfedf2f5d82 |
---|---|
Dependencies: | #30915 |
Reviewers: | Joshua Campbell, Matthias Koeppe → Joshua Campbell, Matthias Koeppe, ... |
New commits:
0b24127 | Merge tag '9.3.beta6' into t/30972/versioned_installation_of_threejs
|
comment:44 Changed 2 years ago by
Cc: | dimpase added |
---|
comment:45 Changed 2 years ago by
Milestone: | sage-9.3 → sage-9.4 |
---|
comment:47 Changed 20 months ago by
Use cases for having several versions of Sage installed
- install new version & keep old one for code you lack time to adapt
- stable release for normal use, beta for development work
- keep several versions around for the sake of bisecting breaking changes
- bare version and version with many optional packages installed
comment:48 Changed 20 months ago by
Commit: | 0b24127485224cd1f4d16f5200132cfedf2f5d82 → 935dcef836f84473e3c3368b0999b4ec310c0eb3 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
935dcef | Merge tag '9.4.beta3' into t/30972/versioned_installation_of_threejs
|
comment:49 Changed 20 months ago by
Replying to paulmasson:
Replying to mkoeppe:
Replying to paulmasson:
Replying to mkoeppe:
That's not a solution because a user may want to use two different Sage versions at the same time.
Why exactly would someone want to do this?
Users have complex needs.
Considering that I don't even use Sage, I'll need a better answer than this before devoting more time to this issue. We are not required to support every feature desired by end users. We can simply tell them to run only one kernel at a time. Who is making the decision to support this option?
for instance one can have two copies of Sage built with different compilers. At least for Sage development purposes it's very normal to have several installations of Sage on a box.
comment:50 follow-up: 51 Changed 20 months ago by
How does one test this with, say, jupyterlab installed by Homebrew?
comment:51 Changed 20 months ago by
Replying to dimpase:
How does one test this with, say, jupyterlab installed by Homebrew?
You would follow the instructions in our manual added in #30476 (https://doc.sagemath.org/html/en/installation/launching.html#setting-up-sagemath-as-a-jupyter-kernel-in-an-existing-jupyter-notebook-or-jupyterlab-installation)
comment:52 Changed 20 months ago by
Unfortunately these instructions are outdated: on macOS I get
% jupyter kernelspec install --user `pwd`/local/share/jupyter/kernels/sagemath Traceback (most recent call last): File "/usr/local/Cellar/jupyterlab/3.0.16/libexec/bin/jupyter-kernelspec", line 33, in <module> sys.exit(load_entry_point('jupyter-client==6.1.12', 'console_scripts', 'jupyter-kernelspec')()) File "/usr/local/Cellar/jupyterlab/3.0.16/libexec/lib/python3.9/site-packages/traitlets/config/application.py", line 845, in launch_instance app.start() File "/usr/local/Cellar/jupyterlab/3.0.16/libexec/lib/python3.9/site-packages/jupyter_client/kernelspecapp.py", line 266, in start return self.subapp.start() File "/usr/local/Cellar/jupyterlab/3.0.16/libexec/lib/python3.9/site-packages/jupyter_client/kernelspecapp.py", line 132, in start self.kernel_spec_manager.install_kernel_spec(self.sourcedir, File "/usr/local/Cellar/jupyterlab/3.0.16/libexec/lib/python3.9/site-packages/jupyter_client/kernelspec.py", line 340, in install_kernel_spec shutil.copytree(source_dir, destination) File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/shutil.py", line 557, in copytree return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks, File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/shutil.py", line 513, in _copytree raise Error(errors) shutil.Error: [('/Users/dima/sage/local/share/jupyter/kernels/sagemath/doc', '/Users/dima/Library/Jupyter/kernels/sagemath/doc', "[Errno 2] No such file or directory: '/Users/dima/sage/local/share/jupyter/kernels/sagemath/doc'")]
comment:54 Changed 20 months ago by
Reviewers: | Joshua Campbell, Matthias Koeppe, ... → Joshua Campbell, Matthias Koeppe, Dima Pasechnik |
---|---|
Status: | needs_review → positive_review |
OK, this works, sorry for noise.
comment:56 Changed 20 months ago by
Branch: | u/mkoeppe/versioned_installation_of_threejs → 935dcef836f84473e3c3368b0999b4ec310c0eb3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
I'm no expert on Python, especially the way implementations my differ, but it can access the local file system and Sage already has
THREEJS_DIR
. Why not set the symlink to Three.js or any package when the kernel initially loads? That way the system package will use whatever version accompanies the kernel.