These are chat archives for ipython/ipython

25th
Nov 2014
Jason Grout
@jasongrout
Nov 25 2014 01:45
@jdfreder?
Jonathan Frederic
@jdfreder
Nov 25 2014 01:50
hey
@jasongrout
Jonathan Frederic
@jdfreder
Nov 25 2014 01:55
You didn't type any other messages did you? Nothing appeared.
Jason Grout
@jasongrout
Nov 25 2014 02:00
check out my comments on ipython/ipython#6990
I think it's almost ready to be reviewed, except I think there is a problem if an accordion has two children with the same model
It's a net gain of 16 lines -- I thought we'd lose some weight here, but oh well.
Jonathan Frederic
@jdfreder
Nov 25 2014 02:14
ok
I will check it out
did your talk go well?
Jason Grout
@jasongrout
Nov 25 2014 16:00
Yes, I think so. It wasn't as prepared as I originally wanted, but that's partly because I was playing around with making a slideshow mode for the notebook :).
Kester Tong
@KesterTong
Nov 25 2014 16:45
is there a dev meeting today? If not, me and @Carreau and other colab devs will be meeting at 9:30 PST. @Carreau, should we do this as an IPython dev meeting (e.g. make public youtube recording, meeting notes etc)?
Jonathan Frederic
@jdfreder
Nov 25 2014 17:09
@jasongrout awesome! :) Being prepared is overrated, I bet the talk went great.
@KesterTong I don't think so, but if you don't mind I'll probably listen in too. If you're broadcasting to youtube I could just use that link so I don't have to take a spot.
Jason Grout
@jasongrout
Nov 25 2014 17:22
@jdfreder: I looked through the widget persistence pr. I got lost about halfway through, so I'm going to come back to it later. But I didn't find the "set_state_callbacks" functions you mention in the what's new portion. Where are those?
Jonathan Frederic
@jdfreder
Nov 25 2014 17:22
A was just responding to you in the PR
Poor communication, sorry.
I meant
That callbacks "that you register" are automatically called when necessary.
With that API, all you have to do is write methods with the signatures load_state() as state dictionary and save_state(state dictionary).
It automatically get_state and set_states for you along with the notebook's save/load.
Alternatively you can get_state and set_state on the widget manager itself, if you want to control when loading/saving happens.
Kester Tong
@KesterTong
Nov 25 2014 17:34
@jdfreder what's your email? I'll add you to the invite
Jonathan Frederic
@jdfreder
Nov 25 2014 17:34
thanks
Jason Grout
@jasongrout
Nov 25 2014 17:35
@jdfreder-So is there a WidgetManager.set_state_callbacks function? I don't see it in the code above. If not, how do I register the load and save callbacks?
Jonathan Frederic
@jdfreder
Nov 25 2014 17:37
@jasongrout oh no :( It looks like the commit is missing
let me see if it's on this machine
I think I wrote it on my laptop
and then wrote the doc on my desktop
probably rebased while I was at it
and didn't pull the latest content
Kyle Kelley
@rgbkrk
Nov 25 2014 18:08
@jdfreder, @Carreau, @KesterTong are you all still meeting?
Jonathan Frederic
@jdfreder
Nov 25 2014 18:09
yes
i think it's closing
Kyle Kelley
@rgbkrk
Nov 25 2014 18:10
okee doke
I'll just work on some stuff. Tempted to merge #6866
Jonathan Frederic
@jdfreder
Nov 25 2014 18:11
@jasongrout I found the commit in my relog
reflog*
Kyle Kelley
@rgbkrk
Nov 25 2014 18:11
Though it looks like @takluyver is still working actually
Jonathan Frederic
@jdfreder
Nov 25 2014 18:14
@KesterTong sorry I dropped early, laptop battery -> 0%
Kyle Kelley
@rgbkrk
Nov 25 2014 18:14
Where is that example kernel that lets you fake your way to a kernel using a subprocess?
Kester Tong
@KesterTong
Nov 25 2014 18:20
@jdfreder no problem, thanks for coming
@rgbkrk just finished
Kyle Kelley
@rgbkrk
Nov 25 2014 18:20
Rad
Kester Tong
@KesterTong
Nov 25 2014 18:21
@rgbkrk I'll send out an email summary and cc you
Kyle Kelley
@rgbkrk
Nov 25 2014 18:21
@KesterTong do you still want to go after remote kernels?
As in, if we maintained the chrome app but you could connect to somewhere, run by us (IPython/Jupyter) or some provider
Curious to hear your thoughts on that stuff, as well as where I can help
Jonathan Frederic
@jdfreder
Nov 25 2014 18:22
@jasongrout sorry about that. I found the missing commit, checked it out, rebased on top of it, and re-added the what's new doc.
Should be ready for your review now.
Kester Tong
@KesterTong
Nov 25 2014 18:23
@rgbkrk do you mean allow the Chrome App to connect to remote kernels?
Kyle Kelley
@rgbkrk
Nov 25 2014 18:24
yup!
I mean, maybe we just have it working in IPython proper
Complete with Google Drive like you've been working on
Sorry, I would have asked in the meeting
I can also do a hangout again
Kester Tong
@KesterTong
Nov 25 2014 18:25
lets chat over hangout
Kyle Kelley
@rgbkrk
Nov 25 2014 18:25
k
Kester Tong
@KesterTong
Nov 25 2014 18:25
give me 2 mins to find a room
Jason Grout
@jasongrout
Nov 25 2014 19:36
@jdfreder?
Jonathan Frederic
@jdfreder
Nov 25 2014 19:37
Hey Jason
fixed the this/that problem you pointed oout
and checked in the code
Jason Grout
@jasongrout
Nov 25 2014 19:37
thanks.
Jonathan Frederic
@jdfreder
Nov 25 2014 19:37
pushed*
Jason Grout
@jasongrout
Nov 25 2014 19:37
is it supposed to now persist when you refresh a page?
Jonathan Frederic
@jdfreder
Nov 25 2014 19:38
Yeah, only if you use the sample script from the whats new doc
Jason Grout
@jasongrout
Nov 25 2014 19:38
oh, I'll try that.
(I'd say enable that automatically... :)_
Jonathan Frederic
@jdfreder
Nov 25 2014 19:38
iow, it won't work unless you've implemented it
we will I think
but I just wanted to get a working API first
then we can debated about what to name the localStorage key etc..
since that will probably be a long standing debate
Jason Grout
@jasongrout
Nov 25 2014 19:39
nope, doesn't work.
Jonathan Frederic
@jdfreder
Nov 25 2014 19:39
(naming always seems to be)
hmph
Jason Grout
@jasongrout
Nov 25 2014 19:40
if I run that in a cell, when I refresh the load isn't there.
Jonathan Frederic
@jdfreder
Nov 25 2014 19:40
maybe there was a commit after that commit that I also lost
let me take a look
and I'll ping you back
these problems are things I've seen before
including the this/that problem
Jason Grout
@jasongrout
Nov 25 2014 19:41
Yeah...how will it know to load if you refresh the page, so it loses that callback because we are starting from scratch?
Jonathan Frederic
@jdfreder
Nov 25 2014 19:41
It will work if your notebook is trusted
the JS gets executed on refresh
reinstalling the callbacks
Jason Grout
@jasongrout
Nov 25 2014 19:43
so I put that example in a cell, explicitly File|Trusted the notebook, and it still didn't work
(and I fixed the this/that issue in my tree, so that isn't the problem)
Jonathan Frederic
@jdfreder
Nov 25 2014 19:44
yeah
there's a missing commit
I've seen this problem before
(I'm looking at the branch right now)
ugh
git
:worried:
Jason Grout
@jasongrout
Nov 25 2014 19:44
When I do %%javascript, is that code output onto the page in a <script> tag?
(so that it is re-executed when the page loads again)?
Jonathan Frederic
@jdfreder
Nov 25 2014 19:45
It does get re-executed on page refresh
iirc we use jquery to append and evaluate it
I think jquery itself creates a script tag to evaluate
@jasongrout it's another that = this problem
I'll push the commit in a second
Jason Grout
@jasongrout
Nov 25 2014 19:50
thanks
Jonathan Frederic
@jdfreder
Nov 25 2014 19:52
@jasongrout Ok, it's pushed. Sorry you must think I'm crazy. Just some weird sync problems that I deserve for working on two different machines at once.
Jason Grout
@jasongrout
Nov 25 2014 19:54
:)
Jonathan Frederic
@jdfreder
Nov 25 2014 19:54
Does it work for you now?
Jason Grout
@jasongrout
Nov 25 2014 19:54
(by the way, I think my viewlists pr is ready for review :)
Jonathan Frederic
@jdfreder
Nov 25 2014 19:55
Ok, I'll review it
now
Jason Grout
@jasongrout
Nov 25 2014 19:59
the Clear Output in the menu should probably remove widgets too, now.
Okay, it looks like there is a problem.
Try making a box with two sliders in it.
Then run your example, then refresh the page.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:01
hmm
something to do with model unpacking I'm guessing
looking...
Jason Grout
@jasongrout
Nov 25 2014 20:01
It appears that the sliders are instantiated for the cell by themselves, and then also instantiated as children of the box
(I'm also testing this on top of my viewlists pr, but I don't think this is the issue)
Jonathan Frederic
@jdfreder
Nov 25 2014 20:01
viewlists will fix this automagically?
Jason Grout
@jasongrout
Nov 25 2014 20:02
no
I think the problem may be that you are instantiating all the views, instead of just the top-level ones
so you instantiate two sliders and a box for the cell, and the box also instantiates two sliders inside of itself
so you get 4 sliders
two inside the box, two next to it.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:03
ahh
I see it
Jason Grout
@jasongrout
Nov 25 2014 20:03
do we distinguish views that are direct children of the top-level cell?
those are the ones to create
Jonathan Frederic
@jdfreder
Nov 25 2014 20:04
No we don't
Jason Grout
@jasongrout
Nov 25 2014 20:04
when a child view is created, the parent view is passed in.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:04
                        // Get the views that are displayed *now*.
                        for (var id in model.views) {
                            if (model.views.hasOwnProperty(id)) {
                                var view = model.views[id];
                                var cell_index = that.notebook.find_cell_index(view.options.cell);
                                state[model_id].views.push(cell_index);
                            }
                        }
Jason Grout
@jasongrout
Nov 25 2014 20:04
perhaps the cell can store a list of its direct children.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:05
that's the code at fault right now
Jason Grout
@jasongrout
Nov 25 2014 20:05
just like any parent view is storing a list of its direct children.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:05
Ah yes
this PR adds that
so what I need to do
it check to see that the view is in that list
simple
one sec..
This should do the trick:
                        // Get the views that are displayed *now*.
                        for (var id in model.views) {
                            if (model.views.hasOwnProperty(id)) {
                                var view = model.views[id];
                                var cell = view.options.cell;

                                // Only store the cell reference if this view is a top level
                                // child of the cell.
                                if (cell.widget_views.indexOf(view) != -1) {
                                    var cell_index = that.notebook.find_cell_index(cell);
                                    state[model_id].views.push(cell_index);
                                }
                            }
                        }
I'll test it, then push
Jonathan Frederic
@jdfreder
Nov 25 2014 20:11
that worked
@jasongrout I pushed the fix
That's one of the cool things of this PR
Jason Grout
@jasongrout
Nov 25 2014 20:13
yep, it seems to work
Jonathan Frederic
@jdfreder
Nov 25 2014 20:13
it moves the display view logic into the codecell
where it belongs
and because of that, it's easy to keep references to the views of the cell
this also had the positive side effect of uncovering some life cycle bugs
which I was able to clean-up
Jason Grout
@jasongrout
Nov 25 2014 20:21
and your cleaning those up exposed a bug in my viewlist pr, which I fixed.
Jonathan Frederic
@jdfreder
Nov 25 2014 20:23
@jasongrout I'm reviewing ipython/ipython#6990 now
Jonathan Frederic
@jdfreder
Nov 25 2014 20:29
@/all I see @Carreau added entries to the Hackpad for a Google Hangout this Thursday which is Thanksgiving Day in the US. Is everyone planning on attending or should I push those and my not-yet-added bullet points back to next week- maybe as a Tuesday meeting???
Thomas Kluyver
@takluyver
Nov 25 2014 20:30
I won't be online. We should probably push it to next week.
Min RK
@minrk
Nov 25 2014 20:30
yeah, push it
Jonathan Frederic
@jdfreder
Nov 25 2014 20:30
Ok, I'll push it to Tuesday, and if there's not enough stuff by then, we can push it to Thursday.
Thomas Kluyver
@takluyver
Nov 25 2014 20:31
sounds fine
Jason Grout
@jasongrout
Nov 25 2014 20:52
@jdfreder: so this should save to the notebook metadata, right?
%%javascript
require(['widgets/js/manager'], function(manager) {
    console.log('loading triggers');
    manager.WidgetManager.set_state_callbacks(function() { // Load
        console.log('loading state', IPython.notebook.metadata.widgets_state);
        return IPython.notebook.metadata.widgets_state || {};
    }, function(state) { // Save
        IPython.notebook.metadata.widgets_state = state;
        console.log('saving state', IPython.notebook.metadata.widgets_state);
    });
});
Jason Grout
@jasongrout
Nov 25 2014 20:57
never mind, it works!
Jonathan Frederic
@jdfreder
Nov 25 2014 20:59
yes it should
I think there's something tricky there though
the event that fires when the notebook is saved is a save success message
so it will need to be saved again
after you set the metadata
Jonathan Frederic
@jdfreder
Nov 25 2014 21:05
i.e. you may need it to be written like this:
%%javascript
require(['widgets/js/manager'], function(manager) {
    console.log('loading triggers');
    var save_lock = false;
    manager.WidgetManager.set_state_callbacks(function() { // Load
        console.log('loading state', IPython.notebook.metadata.widgets_state);
        return IPython.notebook.metadata.widgets_state || {};
    }, function(state) { // Save
        if (!save_lock) {
            save_lock = true;
            try {
                IPython.notebook.metadata.widgets_state = state;
                console.log('saving state', IPython.notebook.metadata.widgets_state);
                IPython.notebook.save_notebook();
            } finally {
                save_lock = false;
            }
        }
    });
});
Alternatively, I could add a new event that fires before save. What do you think?
Jason Grout
@jasongrout
Nov 25 2014 21:05
yes, definitely add a new event before save
Jonathan Frederic
@jdfreder
Nov 25 2014 21:06
I was just trying to minimize my impact on the rest of the code base
but I think you're right
it makes sense to add a new event
Jason Grout
@jasongrout
Nov 25 2014 21:06
there may be other metadata (besides widgets) that we'd like to update in metadata
Jonathan Frederic
@jdfreder
Nov 25 2014 21:09
@jasongrout ok commit pushed, please try
Jason Grout
@jasongrout
Nov 25 2014 21:10
is the high-level callback called, triggered by that?
Jonathan Frederic
@jdfreder
Nov 25 2014 21:10
it should be
now*
(with the commit I pushed a minute ago)
oh whoops
ok
Jason Grout
@jasongrout
Nov 25 2014 21:11
ah, there it is.
Jonathan Frederic
@jdfreder
Nov 25 2014 21:11
git commit --amend
awesome
Jason Grout
@jasongrout
Nov 25 2014 21:12
so when is save and load callbacks called?
Jonathan Frederic
@jdfreder
Nov 25 2014 21:13
Load is called when the kernel connected promise is fulfilled.
Save is called immediately before the notebook is saved (when save_notebook is called).
Jason Grout
@jasongrout
Nov 25 2014 21:14
so what about a page refresh now?
Jonathan Frederic
@jdfreder
Nov 25 2014 21:14
Page refresh should work, as long as you ctrl-s prior to refresh
Jason Grout
@jasongrout
Nov 25 2014 21:14
?
before, it seemed to be saving state after evaluating each cell
hmmm...does it save after each cell evaluation?
Jonathan Frederic
@jdfreder
Nov 25 2014 21:15
hold on, sorry
Brian just called
Jason Grout
@jasongrout
Nov 25 2014 21:27
@jdfreder: it seems that it's more important now to have that broken link icon, given that widget persistence means that there are now many times we might have widgets disconnected from the backend.
Sylvain Corlay
@SylvainCorlay
Nov 25 2014 21:31
a widget-area-level broken link icon?
Jason Grout
@jasongrout
Nov 25 2014 21:32
or per-widget; I'm not sure which is better
the common case is probably all the widgets in a widget area are broken.
Sylvain Corlay
@SylvainCorlay
Nov 25 2014 21:35
a broken icon per widget is a lot
each widget could be responsible to show (or not) that it is disconnected.
(coloring/broken link icon...)
Jonathan Frederic
@jdfreder
Nov 25 2014 21:38
ok back now
our phone call dropped
yes
the 'comm_alive' sttribute exists now
but I haven't done anything with styling yet
btb
call
Jonathan Frederic
@jdfreder
Nov 25 2014 21:59
ok
back
sorry
@jasongrout yes it loads if you re-evaluate the same cell
because for all it knows, you're re-writing the callbacks
but it shouldn't be saving
it should only be saving on autosave or explicit save.
Jason Grout
@jasongrout
Nov 25 2014 22:56
@jdfreder?
another problem with the promises implementation....
Jonathan Frederic
@jdfreder
Nov 25 2014 22:58
Yeah, what's going on?
Jason Grout
@jasongrout
Nov 25 2014 22:59
there are many places where we wrote something like somefunctionthatreturnspromise().then(function() {my stuff}, utils.reject(...)}
but it should be like what you did in the persistence pr:
functionreturningpromise().then(function() {my stuff;}).catch(utils.reject(...))
otherwise, we don't catch errors thrown in the then portion
Jonathan Frederic
@jdfreder
Nov 25 2014 23:02
ah I see
Jason Grout
@jasongrout
Nov 25 2014 23:02
I'm fixing this in my viewlists pr
Jonathan Frederic
@jdfreder
Nov 25 2014 23:02
thanks
I'll take a look when you have it checked in
Jason Grout
@jasongrout
Nov 25 2014 23:02
It looks like you alternated about half and half...
I'm curious why you defaulted to not logging?
(the second argument to utils.reject)
Jonathan Frederic
@jdfreder
Nov 25 2014 23:31
@jasongrout I tried to not log if I knew the method was used by another method that did log.
Alternatively, we could remove the parameter and make it always log.
I thought that was unnecessary though
especially since the error is a "wrapped error"