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

Sessionwork #579

Merged
merged 9 commits into from Aug 10, 2011
Merged

Sessionwork #579

merged 9 commits into from Aug 10, 2011

Conversation

ellisonbg
Copy link
Member

This refactors and adds tests to IPython.zmq.session.Session. The tests can be run with nosetests IPython.zmq.tests.test_session.

  • Renamed the unpack_message method to unserialize.
  • Removed msg_id and msg_type from the top-level message dict (put into headers) throughout the code base.
  • Added tests.
  • Updated the docstrings for Session.

@minrk
Copy link
Member

minrk commented Jul 14, 2011

One thing you might do, since now you are doing many nested dict requests that weren't there before, is add a few msg_id = msg['header']['msg_id'], so you aren't asking for an element of an element of a dict more than you need to.

@ellisonbg
Copy link
Member Author

OK, I have done this in all of the cases where it helps.

On Thu, Jul 14, 2011 at 1:54 PM, minrk
reply@reply.github.com
wrote:

One thing you might do, since now you are doing many nested dict requests that weren't there before, is add a few msg_id = msg['header']['msg_id'], so you aren't asking for an element of an element of a dict more than you need to.

Reply to this email directly or view it on GitHub:
#579 (comment)

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

* I have gone through and looked for instances of ['msg_type'] and
  ['msg_id'] and tried to make sure that I added ['header'] so
  pull the values out of the header.
* But there are many cases where I can't tell if the dict is the
  full message or the header already. This is especially true
  of the msg_id in the parallel db parts of the code.
* Tests pass, but this is scary.
@ellisonbg
Copy link
Member Author

This branch should not be merged until after the 0.11 release.

@ellisonbg
Copy link
Member Author

@minrk: with 0.11 out, can you review this to make sure it is ready to go into master. The htmlnotebook branch is based on this and I would to have this merged so I can follow master again.

@minrk
Copy link
Member

minrk commented Aug 4, 2011

It hasn't changed since I last looked it over, and I just read it over again, so I think it should be fine. Merging this should probably trigger creating the 0.11 maintenance branch.

@ellisonbg ellisonbg merged commit 260396c into ipython:master Aug 10, 2011
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