Skip to content
This repository has been archived by the owner on Jul 30, 2023. It is now read-only.

common code for jlab3 and jlab4 #8

Closed
wants to merge 5 commits into from

Conversation

parmentelat
Copy link

as mentioned in #3, here is a draft version that

  • works with jlab4 (manually tested)
  • should work with jlab3 - NOT TESTED because I am in no position to test on jlab3 (have never used jlab3 actually..)

I have not received any suggestion yet on
https://discourse.jupyter.org/t/jlab-extensions-that-target-both-jlab3-and-jlab4/

so I went ahead on my own and came up with this strategy, that computes a global JLAB4 variable
glad to hear if that is improvable

@parmentelat
Copy link
Author

while I was at it I spent some time trying to smoothen out the code and to get the gh actions to pass
(I have to confess that I'm not at familiar with this gh actions business, so I learned a lot ;-)

when reviewing, please focus on the 1st commit here, which is the only one relevant to the PR
it's just that I still haven't figured out how to trigger the checks locally on my laptop...

@parmentelat
Copy link
Author

btw, in order to please the 'Enforce PR label' workflow, as you guessed I'm sure:
you need to set a label on this PR - I can't
it has to be among
[ 'bug', 'enhancement', 'feature', 'maintenance', 'documentation' ]

@parmentelat
Copy link
Author

in summary:

  • my deepest apologies for the spam that must have ended in your mailer :)
  • and also because I should probably have created a separate PR

the outcome however is:

  • the first commit on this PR is the one that you should review, hopefully it would allow to support jlab3 and jlab4 in the same code
  • I have manually tested that on jlab4 and it seems to work fine
  • I do not know how to test it on jlab3

as far as the other commits:

  • there are a few changes that make a lot of sense, and that help pass more gh actions (dangling links, typescript conventions)
  • I have also artificially bypassed the lint stage, and this should be addressed totally differently, i.e. by changing the code, but I haven't ventured on that path because the changes are going to be massive I'm afraid, so another discussion..
  • you can easily solve the enforce PR label failure
  • I have no idea what goes on with the the check_release workflow
  • also maybe the failing test will talk to you, I don't know either what is going on here

Later on I'll probably need to split all this in two, but this way you can get a glimpse at what the tests look like right now

@parmentelat
Copy link
Author

I went ahead and split this in two, as I should have done in the first place

only the actual jlab3/jlab4 code change remains in this PR, and the rest is in a new PR

sorry for the mess, I'm done now :)

@mwouts
Copy link
Owner

mwouts commented Jun 29, 2023

Hey @parmentelat , thank you so much for working on this! And for providing, in the first place, the port of the extension to Jupyter Lab 4. At the moment I am quite busy but I think it's already a great milestone to have this alpha version of the extension for Jupyter Lab 4.

I feel that I should share a bit more of what I think is the plan with you to avoid causing you too much work.

My preference would be to package, at some point, the extension within jupytext, as is currently the case. There is no urgency to do so, because a) I don't have that much time at the moment, b) the alpha extension works, and c) we might cause trouble to the users that are still with Jupyter Lab 3.

The GitHub actions on this project have been generated by the cookie cutter sccript and I don't know how well they are suited to the extension. I might not take them back to the jupytext project, so you don't need to worry about getting the test passing here.

Last but not least, what I am missing for integrating your changes on the v4 version of the extension in Jupytext, is the packaging part. I have identified four differences:

  1. The separate extension that works (let me call it the "poc") uses hatchling, while jupytext uses an older (and deprecated) framework.
  2. The poc has its files at the root while jupytext has the lab extension at packages/labextension.
  3. The poc uses pyproject.toml while jupytext uses setup.py
  4. jupytext actually builds the extension only if the env variable BUILD_JUPYTERLAB_EXTENSION is set to "1" (the motivation was to keep the build time short, but maybe now we could remove that and always package the extension?)

@parmentelat
Copy link
Author

Hi @mwouts

thanks for the feedback

I had actually not quite realized that the jlab extension was being packaged with a plain pip install jupytext
that actually makes a lot of sense

(but btw, this means that in all my tests until here, I actually had 2 extensions installed, the one that comes with jupytext and the one that comes with jupyterlab-jupytext, I was not aware of that, and I feel lucky because it could have had nastier consequences..)

so OK I understand the goal; on my end the goal is to be able to provide students with installation instructions that are as simple as possible (so typically no pip install git+https://github.com/... ;) and this regardless of the substrate that we choose to run on (nbclassic of nb7, because that's the current question indeed..)
looking at beginning of september as the timeframe

so I'm willing to contribute to achieve this kind of goal,
and I guess the direction where I can be most helpful in the near future would be to test the current code in jlab3 ?
(provided that I understand how to be sure to test the 'right' code in that context, see my remark above)

I can also try to address the packaging differences on the side, I need to get to speed with this pyproject.toml thing too, regardless ;-)

let me know what you think

PS.
can you please also elaborate a bit on why there is one code in jupytext/packages/labextension and another in the separate jupyterlab-jupytext repo ?

checks that app.name is JupyterLab before setting JLAB4 = false
this way if a future app has version 1.x, it will not trigger the jlab3 code
@parmentelat
Copy link
Author

I have been able to test - manually that is - that this code works fine with jlab 3:

btw, this recipe will install jupytext without the labextension, and it is important in order to make sure there is no leftover of the labextension code

conda create -n jlab3-text python=3.11
conda activate jlab3-text
pip install 'jupyterlab < 4.0'
pip install 'git+https://github.com/mwouts/[email protected]'
pip install 'git+https://github.com/parmentelat/jupyterlab-jupytext@jupyterlab4'

I'm only checking that one can successfully double-click to open a text notebook
(I test md:myst and py:percent as far as I am concerned)

; like always I have this in place

cat ~/.jupyter/labconfig/default_setting_overrides.json
{
  "@jupyterlab/docmanager-extension:plugin": {
      "defaultViewers": {
          "python": "Jupytext Notebook",
          "markdown": "Jupytext Notebook"
    }
  }
}

@parmentelat
Copy link
Author

now that I have realized that installing pip install jupytext comes with the old jlab3 extension, and that 2 extensions were coexisting with the same name, it feels like it could explain

I am almost certain of the former (this has been really nagging me for some time)
I have no hard evidence of the latter but I'll monitor this more closely, and report if I find more reasons to believe it was the case

@parmentelat
Copy link
Author

we might have a short-term solution for shipping with the standard jupytext packaging a jlab extension that supports jlab3 and jlab4
mwouts/jupytext#1092

I'm closing this PR, essentially its intrinsic value has been entirely captured in the PR above

@parmentelat parmentelat closed this Jul 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants