-
Notifications
You must be signed in to change notification settings - Fork 174
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix IPython progress bar in notebook versions >= 4 #1088
Conversation
f963e26
to
a3a1fe9
Compare
|
Looked this over and tested it in the newest version of jupyter. Doesn't work in master, works in this branch, so this looks good to me! Will merge on second review... nominating @drasmuss as this is pretty small code-wise and would be good to ensure it works in Windows. |
When I run this I don't get any progress bar, and this error shows up in the console:
Note: that is if I use the version of the notebook from the repo. If I convert the notebook to 4.0 (by saving it) and then run it, there is no error message but still no progress bar. |
Can you give me your |
This is python 3.5, anaconda |
Does it work if you execute |
That command gives |
Install |
Still doesn't work, but new error now:
|
I can't reproduce the problem (using an Anaconda install on Windows 8). How did you install your Python etc? |
All just standard virtualenv setup through Anaconda. It's Windows 10, but I'd be surprised if that made a difference. |
Here's the steps that reproduce the error for me:
|
Can we pull this out of 2.1.1? I'd like to get that released, since it helps me with some Nengo OCL stuff a lot. |
Might as well put it in and do another fix in a later release? |
No, doesn't work in |
Ok, yeah, that makes sense to put it in then. |
@drasmuss: Can you update |
That did the trick, works with ipywidgets 5. Is there a reason that the standard Anyway, I think this is good to merge, as it's at least >= to the behaviour in master. Just two possible comments come to mind:
|
Actually it looks like
|
I'm not sure if the combination of
I will investigate this, but probably they don't state a version and the it fetches the newest version? All my installs end up with version 5 at least.
These are actual requirements for the IPython progress bar.
I don't think so because otherwise Jupyter would already do it? At least it usually prints a warning telling you to execute that command (given you versions match up). Also, if you install jupyter via a package manager this tends to happen automatically by means of the package manager (but that doesn't apply to |
Not mine
|
Seems like conda's jupyter recipe is lagging behind jupyter... I'm inclined not to add anything to requirements as it won't be necessary once conda updates its recipes. Also, worst case is just no progress bar, nothing breaks. So... okay for me to merge then? |
The [Anaconda package list] still lists notebook 4.1.1. Maybe @drasmuss somehow updated notebook without updating ipywidgets? |
You can see all the steps I'm following above, I promise I didn't leave anything out. It's just the standard anaconda jupyter installation. |
The current |
Indeed, should we create an upstream issue? |
Will merge when @jgosmann removes his assignment 馃憤 (squash commit looks good to me btw) |
Sorry, forgot to unassign myself. Go ahead. :) |
428487a
to
c0643d5
Compare
Description:
Updates the Jupyter notebook extension to be compatible with notebook and ipywidget versions >= 4, including notebook 4.2 and ipywidgets 5. Also gives a better exception when
ipywidgets
is not installed.Motivation and context:
They keep changing their API. 馃槨
Fixes #1085. Adresses discussion in #1084 (but not the issue itself).
How has this been tested?
Manually by creating virtualenvs with different IPython/Jupyter/ipywidgets versions.
Where should a reviewer start?
The single file that has been changed. duh
How long should this take to review?
Types of changes:
Checklist: