Skip to content

Port 7.x changes to master #3086

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

Merged
merged 109 commits into from
Feb 10, 2021
Merged

Port 7.x changes to master #3086

merged 109 commits into from
Feb 10, 2021

Conversation

jasongrout
Copy link
Member

@jasongrout jasongrout commented Feb 5, 2021

This merges 7.x into master to pick up changes since we branched 7.x.

Fixes #3085

The basic strategy here was to do a git merge 7.x on master, then fix the conflicts with these ideas:

  • bump any internal javascript alpha version numbers to the next highest major version, to account for the major version release done on the 7.x branch
  • ignore whitespace changes, then run lint at the very end to do formatting cleanups
  • copy over the existing jupyterlab manager ts files from master over the 7.x jupyterlab_widgets package, after making sure that relevant changes from 7.x on those files were already incorporated into master (see the todo item below for some things I noticed doing this)

Checklist

jasongrout and others added 30 commits February 12, 2020 15:25
Update dependencies to JupyterLab 2.0b2 packages
ipywidgets requires notebook format 4.2 or greater to support the mimebundle data generated (in particular, to support the application/…+json mimetype having a json body)

This was exposed by our doc tests, which are now pulling in nbformat package version 5.0, which is more correctly testing the specific notebook version spec for validity.
…port"

This reverts commit 79f6f03, reversing
changes made to ab54ea0.
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
JupyterLab and notebook are upgrading to fontawesome 5 (with the v4 backwards compatibility shim). However, in some places, our CSS is very specific to version 4. This commit modifies these places to use syntax that should for both fontawesome 4 and 5. It involves some DOM structure changes, but that is a private implementation detail, so is backwards compatible with our public API.
Change fontawesome support to work for fontawesome 4 and 5.
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
Bump support for final release of JLab 2
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
Change css variable names to match Lab
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
 - @jupyter-widgets/[email protected]
The 1.4.9 typings cause compilation errors, which are not related to the merge of 7.x into master.
* Ignore jupyterlab_widgets labextension
* Fix eslint config of jupyterlab_widgets to be consistent with top level
* Add some parser options so that eslint can find the correct tsconfig file.
We’ve switched from jqueryui sliders to nouisliders.
@jasongrout jasongrout marked this pull request as ready for review February 5, 2021 21:20
@jasongrout
Copy link
Member Author

I think this is ready for review. I used dev-install.sh to install into a fresh environment and ran most of the doc example notebooks in JupyterLab, and everything looked fine to me.

Once it is reviewed and merged, I can release another 8.0 alpha.

CC @SylvainCorlay

@ianhi
Copy link
Contributor

ianhi commented Feb 7, 2021

Just tried this out. Everything except for the Lorenz example worked great. I'm not sure if that's an issue with the forward port, or with master though, investigating.

image

Copy link
Contributor

@ianhi ianhi left a comment

Choose a reason for hiding this comment

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

The failure above is unrelated - see #3102

I didn't read over all the code here but I tried it out and didn't run into any issues. I also checked and ipympl will run on jlab3 without issue when using this PR so custom widgets may not have so rough of a time 🎉

@jasongrout
Copy link
Member Author

Thanks Ian!

I'll leave this open until tomorrow in case @SylvainCorlay or @vidartf wants to look at the code too.

@davidbrochart
Copy link
Member

ipyleaflet works fine in jlab3 with this PR too.

@vidartf
Copy link
Member

vidartf commented Feb 9, 2021

I tried creating a few branches to see if there were any history problems, but could not find any. I guess the only point of note is that the history of a few files will be a bit "back and forth" due to the cleanup commits, but I guess that should be ok.

@marimeireles
Copy link
Member

Hi @jasongrout, I'm faar from being an expert in ipywidgets, but @SylvainCorlay asked me to check this PR. So, I'll give it a try and run some tests to see if I find something obvs wrong. I'll start now, I can ping you once I'm done.
Maybe @davidbrochart is also taking a look on this?

@marimeireles
Copy link
Member

Oops, sorry, had an old page loaded here.

@vidartf
Copy link
Member

vidartf commented Feb 9, 2021

Hmm. Inspecting the git history of a branch where this is merged into master using a merge-commit, I see one concerning item: This PR will cause tagged commits to be interleaved in the master branch. I'm also seeing a pretty complicated history-tree when using gitk. As such, I'm wondering if this should be merge back into master as a squash commit ? That will make it a little more tricky to navigate the history using git blame though ...

@vidartf
Copy link
Member

vidartf commented Feb 9, 2021

This PR will cause tagged commits to be interleaved in the master branch.

Actually, no, it will only be interleaved in a chronologically sorted history view.

I'm also seeing a pretty complicated history-tree when using gitk.

This might be a good thing, not sure. My git skills aren't up to par.

@jasongrout
Copy link
Member Author

@vidartf - I think on the whole it is good to have the merged 7.x commits as ancestors of master. That will help in, for example, the milestone check script over at #3091 to understand which commits are in 8.x but not in 7.x, etc. It also reflects in the history the actual process of merging in the changes

@jasongrout
Copy link
Member Author

jasongrout commented Feb 9, 2021

I'll start now, I can ping you once I'm done.

@marimeireles - great! I'm ready to merge this (or have someone else merge it), so I can't wait to hear your thoughts before doing so. I plan to release an 8.0 alpha after merging this so we can have a clean base to test and merge new things into.

@marimeireles
Copy link
Member

Hey @jasongrout I tested with ipycytoscape and a few other smaller things. Everything worked for me :)

@jasongrout jasongrout merged commit a3bc13b into jupyter-widgets:master Feb 10, 2021
@jasongrout
Copy link
Member Author

@marimeireles - thanks!

@willingc - this is merged now. I'm planning to make another 8.0 alpha release later this morning or early afternoon. Please let me know if you want me to hold off for one of the docs PRs you've been blocked on.

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.

Port 7.x branch changes to master
8 participants