Backgroundjobs #856

Merged
merged 4 commits into from Oct 16, 2011

Conversation

Projects
None yet
2 participants
@fperez
Member

fperez commented Oct 11, 2011

Updates the backgroundjobs module and adds both a test suite and an illustrative notebook. I actually realized, in the conversation on gh-844, that this was pretty useful code.

I don't have time now to complete bringing the %bg magic back, but that would be pretty easy. But otherwise I think this is useful code.

BTW, right now if one moves to a new cell with a background job printing in an old one, and execute the new cell, futher output from the old bg job shows up in the new cell. It seems to me that we should be able to track the origin of the messages and ensure that they continue appearing in their cell of origin. But I've run out of time to dig into that part of the code, so unless @minrk or @ellisonbg knows off the top of your head how to fix that, we can leave it for later. But it would make for very neat and useful patterns, leaving tasks in the background that continue updating a given cell...

fperez added some commits Oct 10, 2011

Update backgroundjobs lib to current APIs.
This still doesn't bring the %bg magic back in, but it ensures the
basic library works with current code.
Cleanup the API of backgroundjobs to be more convenient to use.
Add support for automatic printing of all tracebacks.

The script at the bottom was removed, as a separate notebook will
illustrate use in a more convenient fashion.
@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 16, 2011

Member

This looks good to me.

I don't think attaching output to cells other than the current one is very simple. The issue is tracking the parent message, which we do at the beginning of execute requests here. Any print/display calls made after that will be attached to that parent, including those made in threads which launched earlier. What you want would require that background threads are not affected by later calls to set_parent(). This essentially means that in the scope of the Thread, shell.displayhook, shell.display_pub, sys.stdout, and sys.stderr must all somehow be different objects from those in the regular scope. I don't know how that's really possible, but the only alternative I see would be to add inspect calls to IOStream.write, which would walk up into some identifying frame to get the appropriate parent on every single call, and have to add extra flushes in case it has pending messages from a previous parent, to prevent collisions.

With the current model, Threads have no attachment whatsoever to the cell that launched them. By launching something in a thread, you are divorcing that code from the cell in which it launched.

Member

minrk commented Oct 16, 2011

This looks good to me.

I don't think attaching output to cells other than the current one is very simple. The issue is tracking the parent message, which we do at the beginning of execute requests here. Any print/display calls made after that will be attached to that parent, including those made in threads which launched earlier. What you want would require that background threads are not affected by later calls to set_parent(). This essentially means that in the scope of the Thread, shell.displayhook, shell.display_pub, sys.stdout, and sys.stderr must all somehow be different objects from those in the regular scope. I don't know how that's really possible, but the only alternative I see would be to add inspect calls to IOStream.write, which would walk up into some identifying frame to get the appropriate parent on every single call, and have to add extra flushes in case it has pending messages from a previous parent, to prevent collisions.

With the current model, Threads have no attachment whatsoever to the cell that launched them. By launching something in a thread, you are divorcing that code from the cell in which it launched.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 16, 2011

Member

Thanks for the review, @minrk. Good analysis on the thread/stdout question, thanks. We'll just table that idea for now: it's not like it's any worse than threads at a terminal, so I have no problem moving on as-is. I'll merge this now then, one less thing to linger.

Member

fperez commented Oct 16, 2011

Thanks for the review, @minrk. Good analysis on the thread/stdout question, thanks. We'll just table that idea for now: it's not like it's any worse than threads at a terminal, so I have no problem moving on as-is. I'll merge this now then, one less thing to linger.

fperez added a commit that referenced this pull request Oct 16, 2011

Merge pull request #856 from fperez/backgroundjobs
Update `IPython.lib.backgroundjobs`: API cleanup, small fixes and improvements, example notebook illustrating their use and test suite.  

Test coverage isn't as good as it should be, but at least we now do have some tests for this code.

@fperez fperez merged commit c40702c into ipython:master Oct 16, 2011

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #856 from fperez/backgroundjobs
Update `IPython.lib.backgroundjobs`: API cleanup, small fixes and improvements, example notebook illustrating their use and test suite.  

Test coverage isn't as good as it should be, but at least we now do have some tests for this code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment