Skip to content
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

extension for jlab[34] : builds under jlab3, packaging untouched #1092

Closed
wants to merge 5 commits into from
Closed

extension for jlab[34] : builds under jlab3, packaging untouched #1092

wants to merge 5 commits into from

Conversation

parmentelat
Copy link
Contributor

@parmentelat parmentelat commented Jun 30, 2023

this is a redeux of #1091 (that I am closing), but without trying to address the packaging at all,
just build the (common) extension on jlab3, exactly like it was with 1.14

this would allow to do a clean release (pip install jupytext) for both targets


successfully built using

pip install 'jupyterlab < 4.0'
pip install jupyter_packaging
git clean -fdx
BUILD_JUPYTERLAB_EXTENSION=1 python setup.py sdist bdist_wheel

so I take it no further change is needed

successfully tested (manually) under jlab3 and jlab4
(as well as under a venv that was jlab3 and then upgraded to jlab4)


again I understand this does not achieve the ultimate goal of refreshing up the packaging...
also the metadata accessing code is ugly as compared to 1.14, but that's the cleanest I could come up with

drop-in replacement of index.ts from
commit 5e2f41231b74ad185fee527330ea38935039dd73
parmentelat/jupyterlab-jupytext@5e2f412
basically one needs to turn off type checking on the jlab4 path now
@mwouts
Copy link
Owner

mwouts commented Jul 2, 2023

Hey @parmentelat , thank you for this! I will give it a try in the coming days, but this sounds really promising. It's really impressive that you've been able to provide something that works both in JLab 3 and 4 👏 No worries at all re the packaging, this is an independent issue that we can address later on.

@parmentelat
Copy link
Contributor Author

imo, despite the erratic path that I have taken, this is the way to go for the near future, in that it's a quick and dirty way to allow users to go on with their lives without having to worry about anything

now, dirty it is indeed, and it's clearly not a long term solution of course since the build runs under jlab3; plus, the code is badly defaced as compared to the original intention... so there is room for improvement, but it does buy us time ;-)

@mwouts
Copy link
Owner

mwouts commented Jul 17, 2023

Hey @parmentelat , my excuses as I have not yet been able to unpile this.

Actually I cannot successfully build the extension anymore, even on main. When I run the commands from your comment above (or equivalently when I use the commands from the documentation), I run into compilation errors. Today the error is:

node_modules/@jupyter/ydoc/node_modules/@lumino/signaling/types/index.d.ts:65:55 - error TS2304: Cannot find name 'AsyncIterable'.

65 export interface IStream<T, U> extends ISignal<T, U>, AsyncIterable<U> {
                                                         ~~~~~~~~~~~~~

node_modules/@jupyter/ydoc/node_modules/@lumino/signaling/types/index.d.ts:289:13 - error TS2339: Property 'asyncIterator' does not exist on type 'SymbolConstructor'.

289     [Symbol.asyncIterator](): AsyncIterableIterator<U>;
                ~~~~~~~~~~~~~

but yesterday I was seeing a different error. Do you have any idea of what could be going on here? Thanks

@parmentelat
Copy link
Contributor Author

hi @mwouts

that's odd because I just tried again and was able to rebuild smoothly
from my - limited - experience with building TS extensions, your errors would resonate with trying to build with jlab4 ?

regardless, just to eliminate any possible discrepancy, let me add that I'm using commit
* 714a82b (HEAD -> main, mine/main) mark version as 1.15.0.dev1

also my virtual env is created using
conda create -y -n jupytext3 python=3.11
conda activate jupytext3

it's of course crucial to install jupyterlab in version 3 with
pip install 'jupyterlab < 4.0'

I can't remember off the top of my head why this is required but I just did
pip install jupyter_packaging

and then the build command that you had provided is unchanged
BUILD_JUPYTERLAB_EXTENSION=1 python setup.py sdist bdist_wheel

I haven't gone beyond that point as part of today's experiment, is that when you experienced the error ?

@mwouts
Copy link
Owner

mwouts commented Jul 17, 2023

Thank you @parmentelat for sharing these details. I still have the same error. How do you install node? (which version do you use?) It's reassuring that you are still able to build the package (BTW the package built still works on the CI too)

@parmentelat
Copy link
Contributor Author

what I see in this virtualenv is this

[jupytext3] jupytext (main $=) $
node --version
v20.3.1

which I inherit from the root install, but that you could, I think, just as well specify with

conda create -y -n jupytext3 python=3.11 node=20

or something to that effect

@parmentelat
Copy link
Contributor Author

imho it would make sense to publish this release to pypi as a dev version (like it is defined in version.py)
even though the ability to pip install from a locally built wheel is very much faster, I would appreciate a way to use that version in remote environments like dockers and the like, plus the ability to let others play with this version using pip install --pre
unless, that is, if you have other experimental features in the pipeline, of course ...

@mwouts
Copy link
Owner

mwouts commented Jul 18, 2023

Hey @parmentelat , I have rebased your PR and merged it. The development version is available on pypi at https://pypi.org/project/jupytext/1.15.0.dev1/, and thanks to the pypi version I have been able to test your PR, finally!

May I let you confirm that it works for you too? If so I will comment on the other PRs to get a few other people to test this dev version, and then we release a non-dev version? What do you think?

@mwouts mwouts closed this Jul 18, 2023
@mwouts
Copy link
Owner

mwouts commented Jul 18, 2023

Also let me thank you again for this PR, indeed having the extension working for both JLab 3 and 4 is amazing!!

@parmentelat
Copy link
Contributor Author

@mwouts
thanks for publishing on pypi !
sure, I will test again the full monty - later this week as I am travelling today
fyi my focus is to ideally get jlab4 + nb7 + jupytext + jupyterlab_myst to work seemlessly together

@parmentelat
Copy link
Contributor Author

@mwouts
I have bad news I'm afraid
please see issue #1107 for details of my first tests of this extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants