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
[Do Not Merge [yet]] Update jupyterlab #67423
Conversation
This change allows previously ~final package sets to be overridden with overlays , e.g. you can do `pkgs.python.pkgs.overrideScope' (self: super: { ... })` whereas previously only `pkgs.python.override { packageOverrides = (self: super: { ... })` was possible. This therefore also allows packages definitions to transitively override dependencies. So e.g. if jupyterlab_server requires jsonschema >= 3, but the rest of the packages should still use jsonschema == 2, you can define it as such: jupyterlab_server = (self.overrideScope' (self: super: { jsonschema = self.jsonschema3; })).callPackage ../development/python-modules/jupyterlab_server { }; This applies the overlay to the whole dependency closure, whereas jupyterlab_server = callPackage ../development/python-modules/jupyterlab_server { jsonschema = self.jsonschema3; }; would only make it apply to jupyterlab_server itself, but not its dependencies, resulting in build-time errors. To get the desired effect without this change, this would be needed for this specific case: jupyterlab_server = callPackage ../development/python-modules/jupyterlab_server { jsonschema = self.jsonschema3; notebook = self.notebook.override { nbformat = self.nbformat.override { jsonschema = self.jsonschema3; }; nbconvert = self.nbconvert.override { nbformat = self.nbformat.override { jsonschema = self.jsonschema3; }; }; }; };
This jsonschema bump came by several times; we can only have a single
version.
…On Sun, Aug 25, 2019, 05:19 Silvan Mosberger ***@***.***> wrote:
@infinisil <https://github.com/Infinisil> requested your review on: #67423
<#67423> Update jupyterlab as a code
owner.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#67423?email_source=notifications&email_token=AAQHZ3335TOVDYRJKUJGZ6LQGH25FA5CNFSM4IPH22FKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOTHQ73QQ#event-2581724610>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAQHZ3YU3GMSY564SA4O4TDQGH25FANCNFSM4IPH22FA>
.
|
@FRidh Why that? We need v3 for jupyterlab which is pretty popular. Would you prefer for v2 to be removed and replaced by v3? |
@GrahamcOfBorg build python37Packages.jupyterlab |
2a1389f
to
29930be
Compare
Ah whoops, I only built jupyterlab_server previously, now jupyterlab itself also works |
@GrahamcOfBorg build python37Packages.jupyterlab |
I seem to be having some initial success trying this approach suggested by @costrouc. Quick/rough experiment looked like this: let
packageOverrides = selfPythonPackages: pythonPackages: {
# v1.3.0 not yet in nixpkgs
nixpkgs-pytools =
selfPythonPackages.callPackage (
{ lib
, buildPythonPackage
, fetchPypi
, jinja2
, setuptools
, isPy27
}:
buildPythonPackage rec {
pname = "nixpkgs-pytools";
version = "1.3.0";
disabled = isPy27;
src = fetchPypi {
inherit pname version;
sha256 = "11skcbi1lf9qcv9j5ikifb4pakhbbygqpcmv3390j7gxsa85cn19";
};
propagatedBuildInputs = [
jinja2
setuptools
selfPythonPackages.rope
];
# tests require network ..
doCheck = false;
}) {};
jsonschema_3_0_2 = pythonPackages.buildPythonPackage rec {
pname = "jsonschema";
version = "3.0.2";
src = pythonPackages.fetchPypi {
inherit pname version;
sha256 = "03anb4ljl624lixrsaxfi7i1iwavw39sd8cfkhcy0dr2dixjnjld";
};
nativeBuildInputs = with selfPythonPackages; [ setuptools_scm nixpkgs-pytools ];
checkInputs = with selfPythonPackages; [ nose mock vcversioner twisted perf ];
propagatedBuildInputs = with selfPythonPackages; [ functools32 attrs pyrsistent ];
# Rewrite imports, alter setup.cfg, replace pkgutil.get in _util.py
postPatch = with selfPythonPackages; ''
substituteInPlace jsonschema/tests/test_jsonschema_test_suite.py \
--replace "python" "${pkgs.python3.pythonForBuild.interpreter}"
substituteInPlace setup.cfg \
--replace 'name = jsonschema' 'name = jsonschema_${src.outputHash}' \
--replace 'jsonschema = schemas/*.json' 'jsonschema_${src.outputHash} = schemas/*.json'
# Unsure if these are necessary
# --replace 'jsonschema = jsonschema.cli:main' 'jsonschema_${src.outputHash} = jsonschema_${src.outputHash}.cli:main'
# --replace 'jsonschema/__init__.py' 'jsonschema_${src.outputHash}/__init__.py'
# --replace 'jsonschema/_reflect.py' 'jsonschema_${src.outputHash}/_reflect.py'
substituteInPlace jsonschema/_utils.py \
--replace 'pkgutil.get_data("jsonschema",' 'pkgutil.get_data("jsonschema_${jsonschema_3_0_2.src.outputHash}",'
python-rewrite-imports \
--path . \
--replace jsonschema jsonschema_${jsonschema_3_0_2.src.outputHash}
'';
checkPhase = ''
nosetests
'';
doCheck = false;
};
json5 = pythonPackages.buildPythonPackage rec {
pname = "json5";
version = "0.8.5";
src = pythonPackages.fetchPypi {
inherit pname version;
sha256 = "1c3k5blbhq7g2lnbap26a846ag5x19ivisd3wfzz6bzdl46hyjqj";
};
doCheck = false;
};
jupyterlab_server = pythonPackages.buildPythonPackage rec {
pname = "jupyterlab_server";
version = "1.0.6";
src = pythonPackages.fetchPypi {
inherit pname version;
sha256 = "1bax8iqwcc5p02h5ysdc48zvx7ll5jfzfsybhb3lfvyfpwkpb5yh";
};
nativeBuildInputs = with selfPythonPackages; [ nixpkgs-pytools ];
propagatedBuildInputs = with selfPythonPackages; [
notebook
json5
jsonschema_3_0_2
];
doCheck = false;
# Substitute the requirement for jsonschema in setup.py and rewrite all imports.
postPatch = with selfPythonPackages; ''
substituteInPlace setup.py --replace "jsonschema>=3.0.1" "jsonschema_${jsonschema_3_0_2.src.outputHash}>=${jsonschema_3_0_2.version}"
python-rewrite-imports \
--path . \
--replace jsonschema jsonschema_${jsonschema_3_0_2.src.outputHash}
'';
}; |
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.
As we've discussed, no overrideScope' within the package set.
And move it to pkgs.jupyterlab_server
And move it to pkgs.jupyterlab
29930be
to
8b0d8d6
Compare
@FRidh I've updated this PR to put I opted to still use python3.pkgs.overrideScope' (self: super: {
jsonschema = self.jsonschema3;
}) vs. what you'd need to write to achieve the same with (python3.override (old: {
packageOverrides = lib.composeExtensions
(old.packageOverrides or (self: super: {}))
(self: super: {
jsonschema = self.jsonschema3;
});
})).pkgs (if I leave out the So I think it's a good idea to merge #67422 because it simplifies this kind of thing a lot. |
@rheno-lindeque and @infinisil the import rewrite method that I've demonstrated will work without overrides. I think it is worth thinking what will happen if we allow incompatible versions of packages into a PYTHONPATH. I could provide a PR that demonstrates how this could be done without overrides? |
@costrouc I'm not sure, but it seems to me that jupyterlab isn't meant to be used as a python package at all. So this PR now just moves it out of the python package set to I'm no python expert though, let me know if I got something mixed up. And feel free to demonstrate your approach if you feel like it fits in nixpkgs, I'm not quite sure myself what this entails. |
I think the link I posted may have gotten lost in the noise. @costrouc posted a detailed description over here:
As a proof point, I did manage to get jupyterlab 1.1.1 working using I suppose a detailed discussion probably wants to go to discourse or an rfc though. I feel as though both approaches have merit, but I'm not a regular contributor. |
Possibly stupid question: with jupyterlab as a standalone package, how does it "see" other Python modules? With FWIW jsonschema is only really used in one place within jupyterlab_server as far as I can see, and is probably easily manually renamed with a bit of sed-magic; that might be the path of least resistance? |
@infinisil , thank you very much for this helpful PR. Do you still work on it? Is it blocked on something specific? |
For one, this PR still depends on #67442, which @FRidh wants docs for, but I really hate writing xml. I could change this PR to not depend on it though by rewriting it in the verbose style described in #67423 (comment). Also I'm not using jupyterlab, and #67423 (comment) mentions that apparently you need to be able to add python modules inside it. I think this PR might break the workflow described there because it can lead to package conflicts for jsonschema. Maybe that's unproblematic though. |
as a heads up, jupyterlab is currently broken on master |
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 this can be simplified greatly now that we have jsonschema-3.1 in nixpkgs. I would suggest retaining this as a python-module, and getting rid of the overlays & various changes to top-level & python-modules/packages that are no longer necessary. Thanks for the work updating this!
@@ -0,0 +1,42 @@ | |||
{ stdenv, buildPythonPackage, fetchPypi, python |
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.
This file is no longer needed as jsonschema has been updated to 3.1 already
@@ -2495,9 +2496,9 @@ in { | |||
|
|||
jupyterlab_launcher = callPackage ../development/python-modules/jupyterlab_launcher { }; | |||
|
|||
jupyterlab_server = callPackage ../development/python-modules/jupyterlab_server { }; | |||
jupyterlab_server = throw "pkgs.python3.pkgs.jupyterlab_server has moved to pkgs.jupyterlab_server"; |
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 we want to keep as a python module for reasons explained in #67423 (comment)
@@ -3113,6 +3114,8 @@ in { | |||
|
|||
jsonschema = callPackage ../development/python-modules/jsonschema { }; | |||
|
|||
jsonschema3 = callPackage ../development/python-modules/jsonschema3 { }; |
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.
no longer needed
@@ -4117,6 +4117,16 @@ in | |||
|
|||
jupyter-kernel = callPackage ../applications/editors/jupyter/kernel.nix { }; | |||
|
|||
inherit ( | |||
let pythonPkgs = python3.pkgs.overrideScope' (self: super: { |
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.
This is no longer necessary now that nixpkgs is on jsonschema-3.1
At work we are developing an extension for JupyterLab. I'd really like to be able to write a nix expression to be able to create a development environment for JupyterLab extensions. We are developing against a post-1.0 version of JupyterLab, so ideally I'd like to use this PR along with @infinisil Is there anything you need help with to get this PR in a state where it can be merged in? |
Likewise wondering if there's anything that can be done to help. |
jupyterlab works fine with 4eed2a5 |
Motivation for this change
Do not merge yet, this is dependent on #67422
Closes #65625
This introduces
jsonschema3
in parallel tojsonschema
(version 2), because apparently a lot of python packages probably still depend on v2Ping @FRidh @JonathanReeve
Things done
nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)