Use skip_if_exists from jupyter-packaging

[martinRenou]

This is not mandatory for the extension to work.

In order to prevent the jupytext-feedstock to build the labextension and the nbextension (hence not depend on jupyterlab and nodejs), we can use jupyter-packaging's skip_if_exists function which allows us to skip the build. In that case we need to ship the nbextension and labextension as part of the sdist.

[mwouts]

Oh interesting! Thank you @martinRenou . Out of curiosity, is there any argument against having jupyterlab and node as dependencies in the build step of jupytext-feedstock? Otherwise I'm fine with rebuilding the extension, it does not seem to be that long.

[martinRenou]

It's just fine keeping things as they are right now 😊

[mwouts]

Well finally @martinRenou I might be interested... I saw that pip install . now has a dependency on nodejs #706 and that seems a bit too much in certain contexts like pre-commit-hooks (I'm sorry, I just read your tweet about them, you're going to dislike them even more 😄 )

So just tell me, with skip_if_exists, can I expect to compile the jupyterlab-jupytext extension (with nodejs) only when we ship a new release of the extension? Will that speed-up pip install . and also remove the dependency on nodejs?

[martinRenou]

that seems a bit too much in certain contexts like pre-commit-hooks

Well, maybe you can get rid of pre-commit hooks 😉

All jokes aside, yes the setup.py now needs yarn (nodejs) and jupyterlab3 to build the labextension. Wasn't it depending on nodejs already? You were building the nbextension and labextension tgz in the setup.py already IIRC.

skip_if_exists will only skip the labextension build if it already exists locally, at least that's the way we used it in other repos like bqplot. And because it can be shipped in the sdist, we can skip the build in the conda-forge feedstock.

What kinds of commands are you willing to run in the pre-commit hooks? Maybe it's not worth running pip install . in there, that would be very slow to execute... You should instead run simple and fast commands like flake8.

[mwouts]

Thanks @martinRenou . Well before jupyter-packaging we had the jupyterlab-jupytext-x.x.x.tar.gz file in the repository. That file did require nodejs to be built, but we were not building it that often, and it was a manual process - not in setup.py.

I'd be happy to learn exactly how skip_if_exists works, and I'll give a look at bqplot. I think I'd like to understand what you're thinking of when you write locally - can that mean "in the repo"?

Finally, a word on the pre-commit hook:

• we intend to run jupytext --sync.
• it is only on the first run of the hook that Jupytext needs to be installed,
• and, I don't know pre-commit hooks that well yet, and in particular I'm not sure of why it is trying to install Jupytext from source rather than from pypi.

[martinRenou]

Do I understand that jupytext installs pre-commit hooks on the user machine? It's not something you use for developing jupytext?

and, I don't know pre-commit hooks that well yet, and in particular I'm not sure of why it is trying to install Jupytext from source rather than from pypi

Why does it need to install jupytext in the first place?

when you write locally - can that mean "in the repo"?

Yes that's what I meant.

[mwouts]

Indeed, the pre-commit hook that we are discussing here is not the one that we use for developing Jupytext. It will be a hook dedicated to Jupyter notebooks and will essentially do jupytext --sync to ensure that .ipynb files and their .py or .md representation remain consistent when committed.

Thanks for pointing out at bqplot. The exemple is very clear, I'll see tonight how I can reuse it:
https://github.com/bqplot/bqplot/blob/3f4526370df7dc0736df23238036719b5fc472f6/setup.py#L78-L101

[martinRenou]

I don't think skip_if_exists will fix your issue then.

Why is the hook installing jupytext? Shouldn't you assume it's already installed in your case?

[JohnPaton]

pre-commit installs hooks in their own sandboxes, from a provided hook repository (in this case mwouts/jupytext). This is a one-ish time action, it doesn't run on every commit. However, since it's installing from source, and the source install now requires this nodejs stuff, the hook itself comes to depend on nodejs which isn't reasonable.

In any case it seems like this should be an optional extra, also for contributors who aren't developing the extension but only the cli (for example). I don't know by which mechanism that can be achieved, though.

[mwouts]

Well if I knew how to pass arguments to setup.py I think I'd add an optional argument --build-jlab-extension. But I don't, unfortunately, so I'm thinking of starting with a simple try/except, and later on we can refactor this into the optional argument...

[mwouts]

Oh I see https://stackoverflow.com/questions/677577/distutils-how-to-pass-a-user-defined-parameter-to-setup-py

[mwouts]

Also, I see that jupyter-packaging does have a --skip-npm argument:
https://github.com/jupyter/jupyter-packaging/blob/defb54b8f0b97451859d32a8c15301565d933c55/jupyter_packaging/setupbase.py#L60-L63

[martinRenou]

However, since it's installing from source, and the source install now requires this nodejs stuff, the hook itself comes to depend on node which isn't reasonable

Could it somewhat be installed from PyPi instead of from sources?

In any case it seems like this should be an optional extra, also for contributors who aren't developing the extension but only the cli

I totally agree with this.

Jupyterlab-lsp is another package with a very similar workflow. This time the setup.py does not depend on nodejs: https://github.com/krassowski/jupyterlab-lsp/blob/master/python_packages/jupyterlab_lsp/setup.py it will simply install the already built labextension. This labextension must be built separately though, but we could think of a mechanism for which jupytext's Python would be installed without the nbextension/labextension.

[JohnPaton]

Could it somewhat be installed from PyPi instead of from sources?

Not afaik, as it uses git for deciding which revision to install and also because it depends on the .pre-commit-hooks.yaml in the repo (I have also been Googling this)

[mwouts]

Locally I am having some success with #708. Do you deem that as acceptable?

[martinRenou]

Looks acceptable to me :)

Just a note that skip_if_exists would solve another problem. This allows skipping the build if the static files (nbextension and labextension) are already there.

[mwouts]

Thank you @martinRenou . Well I've added an if shutil.which("npm") (see #708) and that seems to work well, both on linux and windows. We'll do with that until we find a better way!