search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.31k stars 451 forks source link

Rewrite jupyter kernel installation code using KernelSpecManager, create kernel install script (entry point) #30298

Open mkoeppe opened 4 years ago

mkoeppe commented 4 years ago

We make the following change to sagelib:

References:

Distributing Jupyter Extensions as Python Packages — Jupyter Notebook 6.1.1 documentation https://jupyter-notebook.readthedocs.io/en/stable/examples/Notebook/Distributing%20Jupyter%20Extensions%20as%20Python%20Packages.html

ipykernel/kernelspec.py at f2e9895a6217c53c3912a0fdeb306a8316fd62fd · ipython/ipykernel https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py

ipykernel/setup.py at f2e9895a6217c53c3912a0fdeb306a8316fd62fd · ipython/ipykernel https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/setup.py

Depends on #33855

CC: @dimpase @nbruin @slel @paulmasson @kiwifb @EmmanuelCharpentier @jhpalmieri

Component: build

Keywords: sd111

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

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,7 +1,6 @@
 We make the following changes to `sagelib`:
 - remove the call to `sage.repl.ipython_kernel.install` from the installation steps done by sagelib's `setup.py` -- it is not compatible with modern python packaging methods anyway
 - add a `console_script` that installs a kernel spec (using `sage.repl.ipython_kernel.install`), call it `sage-kernel` (interface similar to: https://ipython.readthedocs.io/en/stable/install/kernel_install.html)
-- Change `sage-notebook` (which implements `sage -n`) to a shell script that invokes the correct jupyter (from system if installed in system, from SAGE_LOCAL if installed there)

 We call `sage-kernel` in `build/pkgs/sagelib/spkg-install` instead.
mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -4,3 +4,4 @@

 We call `sage-kernel` in `build/pkgs/sagelib/spkg-install` instead.

+We follow `ipykernel.kernelspec` (https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py) in using `jupyter_client.kernelspec.KernelSpecManager` for the actual installation.
mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,7 +1,4 @@
-We make the following changes to `sagelib`:
-- remove the call to `sage.repl.ipython_kernel.install` from the installation steps done by sagelib's `setup.py` -- it is not compatible with modern python packaging methods anyway
-- add a `console_script` that installs a kernel spec (using `sage.repl.ipython_kernel.install`), call it `sage-kernel` (interface similar to: https://ipython.readthedocs.io/en/stable/install/kernel_install.html)
+We make the following change to `sagelib`:
+- add a `console_script` that installs a kernel spec (using `sage.repl.ipython_kernel.install`), similar to  `ipykernel.kernelspec` (https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py), using `jupyter_client.kernelspec.KernelSpecManager` for the actual installation.

-We call `sage-kernel` in `build/pkgs/sagelib/spkg-install` instead.

-We follow `ipykernel.kernelspec` (https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py) in using `jupyter_client.kernelspec.KernelSpecManager` for the actual installation.
mkoeppe commented 4 years ago

Dependencies: #30299

mkoeppe commented 4 years ago

Changed dependencies from #30299 to #28197, #30299

nbruin commented 4 years ago
comment:6

If I recall correctly, the main issue that arises if you want to install the sagemath kernel in the default way is that files are copied. That includes the "doc" directory, which is huge. Making it a symlink works much better. Or perhaps the documentation shouldn't be part of the kernelspec in share/jupyter/kernels anyway?

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,17 @@
 We make the following change to `sagelib`:
 - add a `console_script` that installs a kernel spec (using `sage.repl.ipython_kernel.install`), similar to  `ipykernel.kernelspec` (https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py), using `jupyter_client.kernelspec.KernelSpecManager` for the actual installation.

+---

+References:
+
+Distributing Jupyter Extensions as Python Packages — Jupyter Notebook 6.1.1 documentation
+https://jupyter-notebook.readthedocs.io/en/stable/examples/Notebook/Distributing%20Jupyter%20Extensions%20as%20Python%20Packages.html
+
+ipykernel/kernelspec.py at f2e9895a6217c53c3912a0fdeb306a8316fd62fd · ipython/ipykernel
+https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/ipykernel/kernelspec.py
+
+ipykernel/setup.py at f2e9895a6217c53c3912a0fdeb306a8316fd62fd · ipython/ipykernel
+https://github.com/ipython/ipykernel/blob/f2e9895a6217c53c3912a0fdeb306a8316fd62fd/setup.py
+
+
mkoeppe commented 4 years ago
comment:8

Replying to @nbruin:

If I recall correctly, the main issue that arises if you want to install the sagemath kernel in the default way is that files are copied. That includes the "doc" directory, which is huge. Making it a symlink works much better. Or perhaps the documentation shouldn't be part of the kernelspec in share/jupyter/kernels anyway?

Thanks. I see the symlink doc created in $SAGE_LOCAL/share/jupyter/kernels/sagemath. Do we know how this symlink is used?

nbruin commented 4 years ago
comment:9

I think it gets linked in the help menu.

By the looks of it, the standard Python3 kernel populates that help menu with external links to the python website.

I suspect this locally hosted approach was taken to make sagemath more functional in offline settings (which do happen, particularly in more remote places where internet connections aren't so plentiful or are unreliable)

I think the kernelspec is NOT the place to host this doc. Perhaps this was chosen as a spot that's guaranteed to be accessible by the kernel, and possibly a place that's not problematic to serve files from? I'm actually a little surprised: you'd think the kernel config directory is exactly what you DON'T want a web server to serve.

Generally, I'd say serving /usr/local/doc shouldn't be much of a problem, so perhaps just not symlinking and serving SAGE_DOC instead? I guess looking around the jupyter ecosystem a bit to see what solutions other projects come up with ...

mkoeppe commented 4 years ago
comment:10

Thanks. If it turns out that this symlinking business is indispensable, we could go for the following solution:

  1. Use KernelSpecManager to install it (copying)
  2. Query it to reveal the resources directory (e.g., using jupyter kernelspec list --json)
  3. Place the symlink.
mkoeppe commented 4 years ago
comment:11

Replying to @nbruin:

I suspect this locally hosted approach was taken to make sagemath more functional in offline settings (which do happen, particularly in more remote places where internet connections aren't so plentiful or are unreliable)

Yes, and I think it's important to preserve fully offline operation as a feature.

mkoeppe commented 4 years ago
comment:12

I think packaging the built HTML documentation as an nbextension could also be a solution. This would facilitate remote sage kernels as described in #30306 (3).

mkoeppe commented 4 years ago
comment:13

Replying to @mkoeppe:

I think packaging the built HTML documentation as an nbextension could also be a solution. This would facilitate remote sage kernels as described in #30306 (3).

I've expanded on this in #29868.

dimpase commented 4 years ago
comment:14

Replying to @mkoeppe:

Replying to @nbruin:

If I recall correctly, the main issue that arises if you want to install the sagemath kernel in the default way is that files are copied. That includes the "doc" directory, which is huge. Making it a symlink works much better. Or perhaps the documentation shouldn't be part of the kernelspec in share/jupyter/kernels anyway?

Thanks. I see the symlink doc created in $SAGE_LOCAL/share/jupyter/kernels/sagemath. Do we know how this symlink is used?

isn't copying/building the docs and optionally deleting the installation source the normal way to install software docs?

mkoeppe commented 4 years ago
comment:15

See #30306 for the full context of this discussion.

nbruin commented 4 years ago
comment:16

Replying to @dimpase:

isn't copying/building the docs and optionally deleting the installation source the normal way to install software docs?

It's about the location. Sure, installing the docs .../local/share/docs is completely reasonable. That's an installed location, not a source or build location.

Installing the sagemath kernel in jupyter produces a link to it in .../local/share/jupyter/kernels/sagemath. That's a different location (and possibly in a different "local" tree: for instance, the first might be in $SAGE_LOCAL whereas the latter might be in $HOME/.local if someone decides to install the sagemath kernel for the system-jupyter but only for the one user.

Telling the standard jupyter install_kernel machinery to make doc available in the relevant kernels/sagemath would copy it; not symlink. From jupyter's perspective that's probably a reasonable thing and the most reliable, but it's not what we want, because we don't want two copies of the sage doc -- we probably want 0 or 1.

Other kernels do seem to make doclinks available in the help menu, but they are links to the actual documentation hosted by the project; so not a local copy.

It looks like a symlink was an easy solution to make the docs available to jupyter to serve without copying them over. But it seems that having a symlink in the kernels/sagemath directory is a little unusual from a jupyter perspective.

mkoeppe commented 3 years ago
comment:18

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

mkoeppe commented 3 years ago

Changed keywords from none to sd111

mkoeppe commented 3 years ago
comment:19

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

mkoeppe commented 2 years ago

Changed dependencies from #28197, #30299 to #33855

mkoeppe commented 2 years ago
comment:24

From #33855 comment:5:

The kernel might also be missing in the wheels. Best to redo the kernelspec generation/installation similar to ​https://github.com/ipython/ipykernel/blob/main/setup.py#L108

When building a wheel, the symlink doc must be omitted but the other files should be listed as data_files.