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

Repackage Sage's cropped threejs as a pip-installable package (nbextension) jupyter-threejs-sage #30123

Open
mkoeppe opened this issue Jul 12, 2020 · 72 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 12, 2020

As a follow-up to #30972, we provide a pip-installable package (nbextension) that makes (all supported versions of) the threejs-sage offline script available to a Jupyter notebook:

This allows users to set up a system Jupyter notebook in which Sage is fully functional (including offline 3d graphics), without having to manually create symlinks from a Sage installation to the system's nbextensions directory.

see also:

User reports:

Depends on #30972

CC: @dimpase @kliem @paulmasson @jcamp0x2a @nbruin @slel @mjungmath @orlitzky @kiwifb @antonio-rojas @mwageringel

Component: user interface

Keywords: sd111

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/repackage_sage_s_cropped_threejs_as_a_pip_installable_package__nbextension__jupyter_threejs_sage @ 74ade58

Issue created by migration from https://trac.sagemath.org/ticket/30123

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jul 12, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

comment:2

My guess is that the new install script build/pkgs/sagelib/spkg-install poisons environment variables too aggressively.

The code that installs the Jupyter kernel has moved recently to src/sage_setup/command/sage_install.py.

This should be investigated by someone who knows about the notebook.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Jul 14, 2020

comment:3

According to Dima Pasechnik, this can be fixed by sage -b

Indeed...

That is neverthelass a bug that should be fixed.

@jhpalmieri
Copy link
Member

comment:5

Note that sage -b doesn't work any more: it tries to do cd "$SAGE_SRC" && $MAKE, but SAGE_SRC/Makefile.in was removed in #29411. (If you have done an incremental build, the Makefile is still there.)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 16, 2020

comment:6

Replying to @jhpalmieri:

Note that sage -b doesn't work any more: it tries to do cd "$SAGE_SRC" && $MAKE, but SAGE_SRC/Makefile.in was removed in #29411. (If you have done an incremental build, the Makefile is still there.)

Let's fix that in #30153.

@egourgoulhon
Copy link
Member

comment:7

Replying to @mkoeppe:

Replying to @jhpalmieri:

Note that sage -b doesn't work any more: it tries to do cd "$SAGE_SRC" && $MAKE, but SAGE_SRC/Makefile.in was removed in #29411. (If you have done an incremental build, the Makefile is still there.)

Let's fix that in #30153.

As of Sage 9.2.beta7 (which includes #30153), sage -b does no longer work to fix the Jupyter issue.

@egourgoulhon
Copy link
Member

comment:8

FWIW, here is the content of SAGE_ROOT/local/share/jupyter/nbextensions with Sage 9.2.beta7:

lrwxrwxrwx 1 eric eric   19 août   3 15:12 jsmol -> /doesnotexist/jsmol
drwxr-xr-x 2 eric eric 4096 mai   30 17:52 jupyter-js-widgets
lrwxrwxrwx 1 eric eric   21 août   3 15:12 mathjax -> /doesnotexist/mathjax
lrwxrwxrwx 1 eric eric   21 août   3 15:12 threejs -> /doesnotexist/threejs

The doesnotexist directory does not look good...
Changing manually the symbolic links to

lrwxrwxrwx 1 eric eric   46 août   3 17:40 jsmol -> /home/eric/sage/9.2.develop/local/share/jsmol/
drwxr-xr-x 2 eric eric 4096 mai   30 17:52 jupyter-js-widgets
lrwxrwxrwx 1 eric eric   48 août   3 17:40 mathjax -> /home/eric/sage/9.2.develop/local/share/mathjax/
lrwxrwxrwx 1 eric eric   48 août   3 17:40 threejs -> /home/eric/sage/9.2.develop/local/share/threejs/

fixes the Jupyter notebook issue reported in this ticket.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 3, 2020

comment:9

Thanks for investigating this.

This is, as I thought, due to the poisoning of environment variables in the script build/pkgs/sagelib/spkg-install.

The links that created here seem to assume that jsmol, mathjax, and threejs are installed in SAGE_LOCAL.

The correct fix is to create spkg-configure.m4 scripts for these packages so that equivalent system packages can be used -- and to use specific directory variables for determining the targets for these symlinks.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 3, 2020

comment:10

In fact, it seems strange that the sagelib installation script would even install these links. Shouldn't the install script of mathjax install its symlink?

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Jupyter notebook broken in 9.2.beta4 spkg-configure.m4 for jsmol, mathjax, threejs to fix broken Jupyter notebook Aug 3, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 3, 2020

comment:11

Hoping that someone who knows about these javascript things will work on this ticket.

@dimpase
Copy link
Member

dimpase commented Aug 3, 2020

comment:12

for some reason ./sage -i mathjax rebuilds sagelib.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Aug 3, 2020

comment:13

I think this ticket is moving in the wrong direction. The issue is not with the installation of JavaScript packages, it is that they all expect a link to nbextensions to exist. There is an entire install file for Jupyter that sets up the environment for JavaScript packages by a call to its update method. Is this method no longer being called after installing the notebook?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 3, 2020

comment:14

As you can see in comment 8, it is called but the paths are configured wrong.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Aug 4, 2020

comment:15

So the notebook doesn't work because of an explicit mangling of environment variables. Why can't you just unmangle the appropriate directory so that notebook setup works? Oddly enough all of the tests in src/sage/repl/ipython_kernel/install.py pass despite the mangling.

As for using a system Three.js I would definitely advise against that. Mr.doob very often makes changes that are NOT backward compatible. The template that renders Three.js is version dependent: you can't just switch to a newer version and expect it to work. That's why I continue to extract the core of the library and package it here separately from the upstream repository.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 4, 2020

comment:16

Replying to @paulmasson:

Oddly enough all of the tests in src/sage/repl/ipython_kernel/install.py pass despite the mangling.

The mangling happens during install, not during testing.

As for using a system Three.js I would definitely advise against that. Mr.doob very often makes changes that are NOT backward compatible. The template that renders Three.js is version dependent: you can't just switch to a newer version and expect it to work.

That's an important bit of information.

That's why I continue to extract the core of the library and package it here separately from the upstream repository.

You package it where?

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Aug 4, 2020

comment:17

Replying to @mkoeppe:

Replying to @paulmasson:

That's why I continue to extract the core of the library and package it here separately from the upstream repository.

You package it where?

If you look at the upgrade tickets, like #29809, I always attach a zip file containing a small portion of the full library. I've been doing that since I started the viewer, at first just to avoid downloading the full library. There is however a change coming in the new year, in which one of the two critical files we use will officially be removed from the library. That will necessitate either continuing the current process or modifying a local copy of the full library, a process over which we would not be guaranteed control.

Volker has been moving my zip files to the mirrors manually. Do you know if that is also the process for libraries with upstream links?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 4, 2020

comment:18

Replying to @paulmasson:

If you look at the upgrade tickets, like #29809, I always attach a zip file containing a small portion of the full library. [...]

Volker has been moving my zip files to the mirrors manually. Do you know if that is also the process for libraries with upstream links?

Thanks. I am looking at build/pkgs/threejs. Is the spkg-src script current? Is this what you actually use to generate archives?

Since the 9.1 cycle we have a better mechanism for tickets that upgrade spkgs. See https://wiki.sagemath.org/ReleaseTours/sage-9.1#For_developers-1

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 4, 2020

comment:19

Replying to @mkoeppe:

Replying to @paulmasson:

As for using a system Three.js I would definitely advise against that. Mr.doob very often makes changes that are NOT backward compatible. The template that renders Three.js is version dependent: you can't just switch to a newer version and expect it to work.

OK, but is it possible to reliably test for presence of a specific version in the system?

Do you know how distribution packagers handle this? (Debian, Gentoo, ...)

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Aug 4, 2020

comment:21

Replying to @mkoeppe:

Replying to @paulmasson:

If you look at the upgrade tickets, like #29809, I always attach a zip file containing a small portion of the full library. [...]

Volker has been moving my zip files to the mirrors manually. Do you know if that is also the process for libraries with upstream links?

Thanks. I am looking at build/pkgs/threejs. Is the spkg-src script current? Is this what you actually use to generate archives?

That's precisely what I used for the last update.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Aug 4, 2020

comment:22

Replying to @mkoeppe:

Replying to @mkoeppe:

Replying to @paulmasson:

As for using a system Three.js I would definitely advise against that. Mr.doob very often makes changes that are NOT backward compatible. The template that renders Three.js is version dependent: you can't just switch to a newer version and expect it to work.

OK, but is it possible to reliably test for presence of a specific version in the system?

Do you know how distribution packagers handle this? (Debian, Gentoo, ...)

You can search for a REVISION string as is done here. This code was written to ditch installed_packages to help package managers.

On a more general note, what if a user installs a newer package of any sort that creates conflicts with Sage? Do you then force installation of the version needed by Sage?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 4, 2020

comment:23

Replying to @paulmasson:

You can search for a REVISION string as is done here. This code was written to ditch installed_packages to help package managers.

Thanks. So this looks at a local installation (located via the THREEJS_DIR variable)
and then picks specific versions of remote URLs.

When you make updates, do you make simultaneous changes to both sagelib and to the threejs spkg?

On a more general note, what if a user installs a newer package of any sort that creates conflicts with Sage? Do you then force installation of the version needed by Sage?

The spkg-configure mechanism of the Sage distribution inspects installed system packages. We either test for particular features of a library/program, or for a range of versions. For example, currently we accept python3 version 3.6.x and 3.7.x but nothing older nor newer. Whenever a package is found to be installed but not suitable, the Sage distribution installs its own copy.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 4, 2020

comment:24

The standard way to install notebook extensions seems to be using jupyter nbextension. I think we may want to use this (instead of the custom code in sage.repl.ipython_kernel.install).

@kiwifb
Copy link
Member

kiwifb commented Aug 4, 2020

comment:25

Replying to @mkoeppe:

The standard way to install notebook extensions seems to be using jupyter nbextension. I think we may want to use this (instead of the custom code in sage.repl.ipython_kernel.install).

In many ways jupyter nbextension is for personal install - which I guess is OK for vanilla sage. But I don't really know if it is designed well enough to be handled properly by a bona fide package management system.

The only real example I have on top of my head is widgetsnbextension and it uses regular distutils to install the extension.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 29, 2020

comment:64

Replying to @kiwifb:

So it puts a copy in site-package that is "importable" and a copy in share/jupyter/nbextensions.

That's right, same behavior as widgetsnbextension

@kiwifb
Copy link
Member

kiwifb commented Nov 29, 2020

comment:65

Replying to @mkoeppe:

Replying to @kiwifb:

Can we avoid the b1 stuff in the future? What is it even indicating.

That's "beta" 1 (because some metadata may need polishing). This is just the standard Python normalization of version numbers

Can't quite believe how that little bit just destroy the ebuild file format. Just 'b' would pass, but 'b1' with a meaning of 'beta 1', I technically should transform that into _beta1 and then retransform it inside the ebuild. If it is standard python versioning someone needs to do something Gentoo side to allow those names (other than argue that they are wrong or stupid).

@antonio-rojas
Copy link
Contributor

comment:66

Replying to @mkoeppe:

Input from downstream packagers would be very welcome

Tested with the Arch package, everything works. +1 from me.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 29, 2020

comment:67

Replying to @kiwifb:

Replying to @mkoeppe:

Replying to @kiwifb:

Can we avoid the b1 stuff in the future? What is it even indicating.

That's "beta" 1 (because some metadata may need polishing). This is just the standard Python normalization of version numbers

Can't quite believe how that little bit just destroy the ebuild file format. Just 'b' would pass, but 'b1' with a meaning of 'beta 1', I technically should transform that into _beta1 and then retransform it inside the ebuild. If it is standard python versioning someone needs to do something Gentoo side to allow those names (other than argue that they are wrong or stupid).

Hopefully we can quickly proceed to an actual release so that this problem will go away.

@kiwifb
Copy link
Member

kiwifb commented Nov 29, 2020

comment:68

Replying to @mkoeppe:

Replying to @kiwifb:

Replying to @mkoeppe:

Replying to @kiwifb:

Can we avoid the b1 stuff in the future? What is it even indicating.

That's "beta" 1 (because some metadata may need polishing). This is just the standard Python normalization of version numbers

Can't quite believe how that little bit just destroy the ebuild file format. Just 'b' would pass, but 'b1' with a meaning of 'beta 1', I technically should transform that into _beta1 and then retransform it inside the ebuild. If it is standard python versioning someone needs to do something Gentoo side to allow those names (other than argue that they are wrong or stupid).

Hopefully we can quickly proceed to an actual release so that this problem will go away.

That's just venting. I can, and did, work around that.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

Changed keywords from none to sd111

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

comment:69

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

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 8, 2021
@mkoeppe

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Jun 21, 2021

comment:72

please rebase

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 22, 2021

comment:73

Replying to @mkoeppe:

Also, suppose we develop a way so that users can install sagelib using pip, instead of going through the sage distribution. Would it make sense to provide a copy of whatever parts of threejs that sagelib needs as a pip-installable Python package?
Is https://github.com/jupyter-widgets/pythreejs of any relevance in this direction?

As it turns out, it is very relevant; and it would probably be best to redo the Python packaging (jupyter-threejs-sage) as a fork of pythreejs. They already have support for JupyterLab 3

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 27, 2021

comment:75

Opened #32070 (Add package pythreejs 2.3.0).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 19, 2021

comment:76

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants