Expose store_history to execute_request messages. #2184

Merged
merged 4 commits into from Aug 20, 2012

Conversation

Projects
None yet
5 participants
Owner

jasongrout commented Jul 21, 2012

The doc changes should definitely be checked by someone who understands the system better. In particular, is the execution counter incremented if store_history is false, or is that tied to silent?

I'm submitting this because Min said it would probably be a good idea: http://mail.scipy.org/pipermail/ipython-dev/2012-July/009937.html

Owner

minrk commented Jul 21, 2012

execution counter is tied to store_history. silent should just be for disabling displayhook stuff. Since we only exposed a silent arg, we used that in an overloaded way to mean both at the same time.

Do you want to propagate this up to ShellChannel.execute method as well?

Jason Grout added some commits Jul 21, 2012

Owner

jasongrout commented Jul 21, 2012

Like that?

Owner

minrk commented Jul 21, 2012

Perfect, yes (though slight grammar tweak needed in store_history
description).

Owner

jasongrout commented Jul 21, 2012

Better?

Owner

minrk commented Jul 21, 2012

yes, thanks!

@takluyver takluyver commented on the diff Jul 22, 2012

IPython/zmq/ipkernel.py
@@ -331,6 +331,7 @@ def execute_request(self, stream, ident, parent):
content = parent[u'content']
code = content[u'code']
silent = content[u'silent']
+ store_history = content.get(u'store_history', not silent)
@takluyver

takluyver Jul 22, 2012

Owner

The docstrings say that silent=True will force store_history to be False, but it doesn't look like this line will do that - if store_history and silent are both True, store_history will be left as True, I think?

Owner

fperez commented Jul 24, 2012

Test results for commit 60122c0 merged into master
Platform: linux2

  • python2.7: OK
  • python3.2: OK (libraries not available: cython matplotlib oct2py pymongo rpy2 wx wx.aui)

Not available for testing: python2.6

Owner

fperez commented Jul 24, 2012

This is looking pretty clean, any further thoughts on it or should we proceed with merge?

Contributor

bfroehle commented Jul 31, 2012

Looks okay to me too.

@takluyver takluyver commented on the diff Aug 2, 2012

IPython/zmq/kernelmanager.py
@@ -219,7 +219,7 @@ def call_handlers(self, msg):
"""
raise NotImplementedError('call_handlers must be defined in a subclass.')
- def execute(self, code, silent=False,
+ def execute(self, code, silent=False, store_history=True,
@takluyver

takluyver Aug 2, 2012

Owner

In the core, store_history defaults to False, and the code path that executes user input sets it to True. I don't know if it makes sense to do the same here, I just thought it worth mentioning.

@takluyver

takluyver Aug 2, 2012

Owner

I think it makes sense to leave the default as True here, because that maintains backwards compatibility.

Owner

takluyver commented Aug 16, 2012

Looks like everyone was happy with this - I'll merge it tomorrow unless anyone wants to take another look.

Owner

takluyver commented Aug 20, 2012

I guess I forgot about it after my last message, but since there were no objections, I'll merge it now.

takluyver merged commit f56e91c into ipython:master Aug 20, 2012

jasongrout deleted the jasongrout:store-history branch Apr 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment