Skip to content
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

Backgroundjobs #856

Merged
merged 4 commits into from Oct 16, 2011
Merged

Backgroundjobs #856

merged 4 commits into from Oct 16, 2011

Conversation

fperez
Copy link
Member

@fperez 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...

This still doesn't bring the %bg magic back in, but it ensures the
basic library works with current code.
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
Copy link
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
Copy link
Member Author

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
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
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants