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

Update for jupyterlab3 - switch to pip install (federated) #19

Merged
merged 12 commits into from
Jan 4, 2021

Conversation

ianhi
Copy link
Collaborator

@ianhi ianhi commented Dec 11, 2020

Updates for jlab3. I took the opportunity to switch this over to the new style "federated" extensions that eliminate the need for a build step. A consequence of this is that a pypi package will need to be released and that the method for loading the vim keymap switched from importing it to calling the ensureVimKeymap function (more details in commit message).

I also applied the recommended tsconfig, eslint, and prettier settings from the upgrade script and then linted in a separate commit.

Closes: #14

Installing now
If anyone is realllllllly wanting this extension for jlab3 prior this being merged then you're in luck @axelfahy has uploaded this PR to pypi and you can install like so

pip install jupyterlab-vim
  • Add a line to History.md briefly documenting the change.

If this is a release, additionally do the following:

  • Update the dependencies in package.json
  • Run jlpm install to update yarn.lock

README.md Outdated

* JupyterLab 2.0
```bash
pip install ????????????
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what name this will end up with on pypi so ??? for now.

Copy link
Collaborator

@axelfahy axelfahy Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either jupyterlab_vim or jupyterlab-vim. - seems to be more used than _ for package names. But I don't think it is a good idea to have the npm package with a _ and the pypi package with a -. Either we keep the underscore or we change for - everywhere? What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like consistency everywhere. Also if you name the python package with an _ than you still pip install using -, I guess pip just substitutes them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know about the substitution, then it might be easier to go with jupyterlab_vim.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 11, 2020

CI fails but I can confirm that the extension works.

@ianhi ianhi mentioned this pull request Dec 11, 2020
Copy link
Collaborator

@axelfahy axelfahy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the update to jupyterlab3! Once we decide on the package name, I will publish this to pypi. But I think we should wait until the release of the final jupyterlab3 version to merge in order to keep compatibility with jupyterlab2 until the stable release of jupyterlab3.

"clean:lib": "rimraf lib tsconfig.tsbuildinfo",
"clean:labextension": "rimraf jupyterlab_vim/labextension",
"clean:all": "jlpm run clean:lib && jlpm run clean:labextension",
"eslint": "eslint . --ext .ts,.tsx --fix",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the build is failing in the pull request because it does not find lint. We need to change lint to eslint in .github/workflows/pull-request.yml.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

also we need to ensure that jupyterlab 3 is being installed that's what is causing the build to fail

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 12, 2020

But I think we should wait until the release of the final jupyterlab3 version to merge in order to keep compatibility with jupyterlab2 until the stable release of jupyterlab3.

Totally agree, but I knew how to do this and to avoid the gotcha of not knowing about ensureVimKeymap so I just wanted to make this public sooner rather than later.

Once we decide on the package name, I will publish this to pypi.

Two things here if it ends up with a name like jupyterlab-vim:

  1. @jwkvam should have an input (or ability to say "please don't")
  2. Given that this is a pretty general name would you be open to giving a few other people write access/pypi access so the project is robust against any one person no longer being involved. (i.e. safegaurd against the jlab2 switch nightmare)?
    • Though if jlab-vim makes it's way into core in 3.1 this would be less of an issue.

@axelfahy
Copy link
Collaborator

2\. Given that this is a pretty general name would you be open to giving a few other people write access/pypi access so the project is robust against any one person no longer being involved. (i.e. safegaurd against the jlab2 switch nightmare)?

Sure, I can set multiple maintainers for the pip repository.

* Though if jlab-vim makes it's way into core in 3.1 this would be less of an issue.

That would be the best solution IMHO, I hope it will be in core in the 3.1 version.

Two things here if it ends up with a name like jupyterlab-vim:

1. @jwkvam should have an input (or ability to say "please don't")

So let's stick with jupyterlab_vim?

following https://jupyterlab.readthedocs.io/en/latest/developer/extension_migration_2_3.html

By making the extension pip installable (federated) the way to access codemirror is a bit different as the extension is no longer included in the jupyterlab build step. So we use the `ensureVimKeymap` function on the ICodeMirror interface. This ensures that the codemirror used by jupyterlab has loaded it's vim bindings. We then access that codemirror via the ICodeMirror interface.
Set eslint and prettier to the defaults in the extension cookiecutter during the upgrade, this applies them to index.ts.
@ianhi
Copy link
Collaborator Author

ianhi commented Dec 14, 2020

I wonder what was so bad about nodejs 13? weird.

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 14, 2020

Looking at this more carefully: the build.yml workflow actually does everything that the pull-request action does already so maybe just delete the old pull-request script and rename the build one?

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 14, 2020

Also, we can set up automated releases to pypi - though the version I know how to use requires you to bump the version manually then you create a release using the github interface. Thoughts on disabling the automated version bumper and switching to script that publishes to both pypi and npm?

@axelfahy
Copy link
Collaborator

Also, we can set up automated releases to pypi - though the version I know how to use requires you to bump the version manually then you create a release using the github interface. Thoughts on disabling the automated version bumper and switching to script that publishes to both pypi and npm?

For Python, I know versioneer, that uses the tag of the commit. But we can bump it manually, there will probably not be many versions.

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 14, 2020

Actually it seems that the single source of truth for versioning is contained in the package.json. _version.py just reads that file to get the version. So The automated version bump should still work, though it will need to be modified to also push to pypi

@axelfahy
Copy link
Collaborator

axelfahy commented Dec 15, 2020

though it will need to be modified to also push to pypi

Yes, definitely. In the meantime, I pushed the package from this branch to pypi (https://pypi.org/project/jupyterlab-vim/). An hyphen was set instead of the underscore on the Pypi page, probably because the main folder has one, even if the package name has undescore (jupyterlab_vim-0.13.0-py3-none-any.whl). I sent you an invite for the pypi project, so you can push new versions as well.

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 15, 2020

Yes, definitely. In the meantime, I pushed the package from this branch to pypi (https://pypi.org/project/jupyterlab-vim/)

Nice! I'll edit my first post to reflect this.

I sent you an invite for the pypi project, so you can push new versions as well.

Awesome thanks. Though hopefully I'll never have to make use of that ability.

Comment on lines 19 to 25
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install setuptools wheel
- name: Build package
run: |
python setup.py sdist bdist_wheel
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will break right now as I'm pretty sure that the build step will still require using nodejs.

The jupyterlab-git version is probably good to base this off of https://github.com/jupyterlab/jupyterlab-git/blob/master/.github/workflows/publish.yml

@ianhi
Copy link
Collaborator Author

ianhi commented Dec 30, 2020

Hey @axelfahy heads up that jlab3 has been released now. Is there anything more we need to do here?

@axelfahy
Copy link
Collaborator

axelfahy commented Jan 4, 2021

Hey @axelfahy heads up that jlab3 has been released now. Is there anything more we need to do here?

I don't think so, I will merge. Thank you!

@axelfahy axelfahy merged commit 344809c into jupyterlab-contrib:master Jan 4, 2021
@ianhi ianhi deleted the jlab3 branch January 4, 2021 15:07
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.

Jupyterlab 3
2 participants