Segregating the widgets (the Small Split) #34
Conversation
jdfreder
commented
Apr 20, 2015
jdfreder
commented
- tests
ipython_widgets/install.py
Outdated
There was a problem hiding this comment.
user=False shouldn't be hardcoded
|
Async rabbit for you! 🐰 |
|
This is ready for review/merge |
jupyter_notebook/notebookapp.py
Outdated
There was a problem hiding this comment.
I though it looked like settings was being used as a generic budle of things, but I must have been imagining it. I'll set it on self.
There was a problem hiding this comment.
I was mainly asking because we log everywhere, and don't need access in
settings, so I was dubious that it actually needed to be added.
On Mon, Apr 20, 2015 at 5:16 PM, Jonathan Frederic <[email protected]
wrote:
In jupyter_notebook/notebookapp.py
#34 (comment)
:@@ -203,6 +241,7 @@ def init_settings(self, ipython_app, kernel_manager, contents_manager,
config=ipython_app.config,
jinja2_env=env,
terminals_available=False, # Set later if terminals are available
log=log,[image: ✔️]
—
Reply to this email directly or view it on GitHub
https://github.com/jupyter/jupyter_notebook/pull/34/files#r28741120.
|
@minrk the last commit greatly simplifies the deprecation handler. |
|
I think I can still make it one handler... One second. |
jupyter_notebook/notebookapp.py
Outdated
There was a problem hiding this comment.
Don't check for or serve the file, you can just redirect.
There was a problem hiding this comment.
Ah, but then my deprecation warning doesn't show.
jupyter_notebook/notebookapp.py
Outdated
There was a problem hiding this comment.
@minrk this is driving me crazy- for the case of http://localhost:8888/deprecatedwidgets/js/init.js this yields:
console.warn('`/static/widgets/js` is deprecated. Use `/nbextensions/widgets/widgets/js` instead.');
define(['/nbextensions/widgets/widgets/js/init.js'], function(x) { console.log('...loaded...'); return x; });In the notebook, when I require this file, the warning shows in the console. After the ~5sec require js timeout, a load timeout error is shown, no 404s in the network panel of the browser debugging tools, and the "...loaded..." text is never printed. HOWEVER, when I try to access the file manually by navigating to http://localhost:8888/nbextensions/widgets/widgets/js/init.js it loads just fine!
There was a problem hiding this comment.
I have a feeling it's the last bullet here :http://requirejs.org/docs/errors.html#timeout
jdfreder
force-pushed
the
smallsplit
branch
4 times, most recently
from
c5c6442 to
797893d
Compare
|
@minrk I moved the handler class def out of the closure. The problem causing the require js timeout was the import paths in /ipython_widgets/static/notebook/js/extension.js not matching the path(s) written by the deprecation warning. I'm not satisfied with the current state of things, which requires a TL;DR This warning is now working, but I want to tweak somethings before we think about merging. |
|
/me sighs I must have been tired last night - didn't notice the |
This is a combination of 9 commits. Shim IPython.html.widgets Add flags to widgets install. Put shim in IPython instead Add deprecation warning to Javascript source Move tests Don't use setting to pass log arround. Simplify deprecated handler. Use requirejs to perform redirect.
|
I squashed the last 8 commits, which all related to the JS deprecation warning, making this PR more lean. |
|
@jdfreder sorry for the struggle. This is looking good, now. |
|
@jdfreder almost there. Can you remove |
|
Yes, that brings up a good point - after the new repository, and your history cleaning, I'll need to do something about the CSS... |
|
I suspect we'll want to add require-css or similar. |
✔️ |
Segregating the widgets (the Small Split)
|
@jdfreder thanks. I've started the actual split to create the widgets repo, but I have somewhere I need to be this evening, so it probably won't finish before tomorrow. |
|
No rush, thanks for doing that! : ) |
Segregating the widgets (the Small Split)
3 participants
