-
-
Notifications
You must be signed in to change notification settings - Fork 654
explictly install jupyter kernel, harmonise with meson #40700
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
base: develop
Are you sure you want to change the base?
Conversation
as #40633 was merged about an hour ago, this will need to revert reverts done there, once the beta1 dust settles |
here ./configure installs the kernel data (filling the version in) into local/share/notebook/ |
Documentation preview for this PR (built with commit 7df7430; changes) is ready! 🎉 |
As well, we make consistent use of PACKAGE_VERSION (instead of SAGE_VERSION) in meson, and rename PACKAGE_VERSION in meson.build's to something more descriptive and non-clashing
#39030 fixes this as well, or not? |
@tobiasdiez I think by default In a different scenario, with external jupyter, As well, in sage-distro the version of sagelib is I must say I don't know what your version setting to |
Certainly using |
@@ -12,7 +12,7 @@ mpfr = dependency('mpfr', version: '>= 4.0.1') | |||
|
|||
# Configuration data | |||
conf = configuration_data() | |||
conf.set('PACKAGE_VERSION', '"' + meson.project_version() + '"') | |||
conf.set('SAGE_MESON_PKG_VERSION', '"' + meson.project_version() + '"') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the version of mpfi, you probably don't want to change that.
@@ -2,7 +2,7 @@ sage_install_dir = py.get_install_dir() / 'sage' | |||
|
|||
# Generate the configuration file | |||
conf_data = configuration_data() | |||
conf_data.set('PACKAGE_VERSION', '1.2.3') | |||
conf_data.set('SAGE_MESON_PKG_VERSION', '1.2.3') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just delete that line along with it's usage in _conf.py.in
. No idea about it's purpose, if it ever served one.
Meson should already install the jupyter kernel in whatever python venv it ends up being installed. The workaround in #39030 was only for editable installs where this was not working correctly. Not sure if user or prefix is better for such editable installs. |
@tobiasdiez @kiwifb |
where in the code does this happen? |
So? According to my tests, the branch here does not fix the broken live doc (broken perhaps because of missing sagemath kernel). |
Downgrading setuptools does not work either in meson build. Recall that it worked with make build. |
I wonder if you are aware that But I don't know about the meson side. |
sage_setup should not be needed in the meson build, as far as I understand - right, @tobiasdiez ? note that the explicit |
SageKernelSpec class is 10 years old and probably hopelessly outdated. I guess modern Jupyter, which has evolved a lot, needs very little there. |
meson build in #39030 uses |
Yes, it's not used for meson and can be deleted after we fully migrated to meson. This is how the Jupyter kernel is installed with meson: Lines 140 to 154 in e656b82
As for the process, are you okay waiting for #39030 to be merged first? Then we can iterate more easily here on the jupyter kernel installation and make sure it doesn't break in one of the 4 supported build methods (plain meson/sage-distro + editable/non-editable). |
@tobiasdiez - absolutely, #39030 should be 1st. Actually, quite a bit of stuff dealing with Jupiter in Sage is totally antique - what nowadays is done by 1-liner |
Sorry I am too unfamiliar with the meson build to be of any help here. Simply, may I suggest that when testing the SageMath kernel, you run Jupyterlab on this notebook: https://github.com/egourgoulhon/SageMathTest/blob/master/Notebooks/test_display.ipynb |
Do you mean Sage's Jupyterlab? External Jupyterlab? |
A priori Sage's Jupyterlab, but it would be nice if the built SageMath kernel works in the system's Jupyterlab as well. |
this notebook basically works fine, save for the test to show a pdf in an external viewer. |
I have tested these changes in Fedora 42 (classic build). It works with
PS. And
|
10.8.beta0 broke installation of sagemath's jupyter kernel, apparently by upgrading setuptools (sic!),
see #40586 (comment)
and #40633
Here we implement the explicit installation of the kernel in the classic build, and make it compatible with meson
📝 Checklist
⌛ Dependencies
#39030 - meson build for sagelib