add qt config option to clear_on_kernel_restart #1681

Merged
merged 4 commits into from May 16, 2012

Conversation

Projects
None yet
5 participants
@ivanov
Member

ivanov commented May 2, 2012

Previously, restarting a kernel in the qtconsole always cleared the
entire session. Since qtconsole has a %clear command, it makes sense to
allow users to decouple the two activities - the restarting of kernels,
and clearing the console. This PR adds a configuration option which allows for the restarting of kernels without losing console history. The default value for clear_on_kernel_restart is set to True to stay consistent with the way things worked before this functionality was added.

To test, run:

ipython qtconsole --IPythonWidget.clear_on_kernel_restart=False

Here's what it looks like when the user restarts the kernel on In[3]:

Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41)
Type "copyright", "credits" or "license" for more information.

IPython 0.13.dev -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.
%guiref   -> A brief reference about the graphical user interface.

In [1]: 0x42
Out[1]: 66

In [2]: 042
Out[2]: 34

In [3]: # restarting kernel...
#------------------------------------------

In [1]:
@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 2, 2012

Member

I could use feedback if I should go ahead and remove the messagebox logic on after line 522 to also use this new option.

Member

ivanov commented May 2, 2012

I could use feedback if I should go ahead and remove the messagebox logic on after line 522 to also use this new option.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez May 2, 2012

Member

No time to review right now (gotta catch a flight early tomorrow, will be offline for a bit), but I wanted to say thank you for this. I've been wanting this for a long time.

Member

fperez commented May 2, 2012

No time to review right now (gotta catch a flight early tomorrow, will be offline for a bit), but I wanted to say thank you for this. I've been wanting this for a long time.

add qt config option to clear_on_kernel_restart
Previously, restarting a kernel in the qtconsole always cleared the
entire session. Since qtconsole has a %clear command, it makes sense to
allow users to decouple the two activities - the restarting of kernels,
and clearing the console.  This commit adds a configuration option which
allows for the restarting of kernels without losing console history. The
default value for clear_on_kernel_restart is set to True to stay
consistent with the way things worked before this functionality was
added.

To test, run:

    ipython qtconsole --IPythonWidget.clear_on_kernel_restart=False

Here's what it looks like when the user restarts the kernel on In[3]:

 Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41)
 Type "copyright", "credits" or "license" for more information.

 IPython 0.13.dev -- An enhanced Interactive Python.
 ?         -> Introduction and overview of IPython's features.
 %quickref -> Quick reference.
 help      -> Python's own help system.
 object?   -> Details about 'object', use 'object??' for extra details.
 %guiref   -> A brief reference about the graphical user interface.

 In [1]: 0x42
 Out[1]: 66

 In [2]: 042
 Out[2]: 34

 In [3]: # restarting kernel...
 #------------------------------------------
 In [1]:
@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 2, 2012

Member

IIRC, we're using a rich text widget - is it easy to draw a proper horizontal line, rather than an ASCII art -----?

Member

takluyver commented May 2, 2012

IIRC, we're using a rich text widget - is it easy to draw a proper horizontal line, rather than an ASCII art -----?

@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 5, 2012

Member

ok, now using html <hr> instead of ASCII ----, took me a bit to get to the bottom of it, so @takluyver better be happy with the results! ;)

Member

ivanov commented May 5, 2012

ok, now using html <hr> instead of ASCII ----, took me a bit to get to the bottom of it, so @takluyver better be happy with the results! ;)

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 5, 2012

Member

Thanks Paul, I've just tested, and I like the way it's looking now.

Member

takluyver commented May 5, 2012

Thanks Paul, I've just tested, and I like the way it's looking now.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk May 13, 2012

Member

Looks good to me. Any reason not to merge?

Member

minrk commented May 13, 2012

Looks good to me. Any reason not to merge?

@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 14, 2012

Member

@minrk, actually, yeah, hang on, found a minor problem with this

Member

ivanov commented May 14, 2012

@minrk, actually, yeah, hang on, found a minor problem with this

move clear_on_kernel_restart back inside reset()
I originally made a new method (reset_on_restart) to accommodate this
new functionality, but it turns out I ended up needing to replicate
almost everything that reset() already does inside this new method. In
particular, reset_on_restart did not work properly when restarting a
kernel that was in debug mode.

Everything should now work, and we don't have another method to deal
with.
@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 15, 2012

Member

ok @minrk and @takluyver - I think it's ready to merge again:

I originally made a new method (reset_on_restart) to accommodate this
new functionality, but it turns out I ended up needing to replicate
almost everything that reset() already does inside this new method. In
particular, reset_on_restart did not work properly when restarting a
kernel that was in debug mode.

Everything should now work, and we don't have another method to deal
with.

Member

ivanov commented May 15, 2012

ok @minrk and @takluyver - I think it's ready to merge again:

I originally made a new method (reset_on_restart) to accommodate this
new functionality, but it turns out I ended up needing to replicate
almost everything that reset() already does inside this new method. In
particular, reset_on_restart did not work properly when restarting a
kernel that was in debug mode.

Everything should now work, and we don't have another method to deal
with.

@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 15, 2012

Member

having second thoughts about calling the parameter force, does clear make more sense? or force_clear?

Member

ivanov commented May 15, 2012

having second thoughts about calling the parameter force, does clear make more sense? or force_clear?

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk May 15, 2012

Member

clear seems most appropriate, since that's the switch you are flipping.

Member

minrk commented May 15, 2012

clear seems most appropriate, since that's the switch you are flipping.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 15, 2012

Member

I agree that clear makes more sense - force sounds like "don't ask for confirmation".

Other than that, it looks good, and I think putting it inside the reset method makes sense. I assume you've tested it again in both modes.

Member

takluyver commented May 15, 2012

I agree that clear makes more sense - force sounds like "don't ask for confirmation".

Other than that, it looks good, and I think putting it inside the reset method makes sense. I assume you've tested it again in both modes.

@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov May 15, 2012

Member

ok, thanks for being patient with me, guys. Renamed it to clear, ready to merge, and yes, tested it in both modes.

Member

ivanov commented May 15, 2012

ok, thanks for being patient with me, guys. Renamed it to clear, ready to merge, and yes, tested it in both modes.

ivanov added a commit that referenced this pull request May 16, 2012

Merge pull request #1681 from ivanov/optionally-clear-on-restart
add qt config option to clear_on_kernel_restart.

This allows for restarting kernels without clearing the qtconsole,
while leaving a visible indication that the kernel has restarted.

@ivanov ivanov merged commit cffb287 into ipython:master May 16, 2012

@SamuelDeleglise

This comment has been minimized.

Show comment
Hide comment
@SamuelDeleglise

SamuelDeleglise Oct 6, 2014

Hi all,

I am not sure if this is the best place for this comment, but I will try anyway:
I use ipython notebook (IPython 2.1.0) on windows. The options are interpreted correctly (for instance launching the notebook with --IPythonWidget.confirm_restart=False has the correct effect), however, the option IPythonWidget.clear_on_kernel_restart=True has no effect. This is particularly cumbersome since there is a small bug associated with restarting the kernel without clearing the screen: the console doesn't scroll down to the first command prompt (it scrolls down only to the horizontal line, and I have to hit enter once to have it scroll to the new command prompt).

Are these 2 bugs (scrolling problem + no way to disable clear_on_kernel_restart) windows related and is there a way to solve them? I use the pythonxy distribution and these problems have been appearing on all the computers I have tried so far.

Thanks for your help,
Samuel

Hi all,

I am not sure if this is the best place for this comment, but I will try anyway:
I use ipython notebook (IPython 2.1.0) on windows. The options are interpreted correctly (for instance launching the notebook with --IPythonWidget.confirm_restart=False has the correct effect), however, the option IPythonWidget.clear_on_kernel_restart=True has no effect. This is particularly cumbersome since there is a small bug associated with restarting the kernel without clearing the screen: the console doesn't scroll down to the first command prompt (it scrolls down only to the horizontal line, and I have to hit enter once to have it scroll to the new command prompt).

Are these 2 bugs (scrolling problem + no way to disable clear_on_kernel_restart) windows related and is there a way to solve them? I use the pythonxy distribution and these problems have been appearing on all the computers I have tried so far.

Thanks for your help,
Samuel

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 6, 2014

Member

Hi Samuel - it looks like the code to check that option was taken out somewhere along the line. If you can work out where it should be checked, please make a pull request to do so.

Member

takluyver commented Oct 6, 2014

Hi Samuel - it looks like the code to check that option was taken out somewhere along the line. If you can work out where it should be checked, please make a pull request to do so.

@SamuelDeleglise

This comment has been minimized.

Show comment
Hide comment
@SamuelDeleglise

SamuelDeleglise Oct 6, 2014

Hi Thomas, Thanks for the quick reply, I have been looking around in the code, especially in
IPython\qt\console\frontend_widget.py
Hoping for a commented line or something as trivial as that, but after one hour of digging, I must admit my incompetence. I tried to add a self.reset(True) in restart_kernel(..) but I have the feeling it's actually restarting the kernel twice since the restart command is taking very long...
Sorry for not being more helpful,
Cheers,
Samuel

Hi Thomas, Thanks for the quick reply, I have been looking around in the code, especially in
IPython\qt\console\frontend_widget.py
Hoping for a commented line or something as trivial as that, but after one hour of digging, I must admit my incompetence. I tried to add a self.reset(True) in restart_kernel(..) but I have the feeling it's actually restarting the kernel twice since the restart command is taking very long...
Sorry for not being more helpful,
Cheers,
Samuel

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 6, 2014

Member

It certainly looks like sticking self.reset(True) in the right part of restart_kernel (about where it adds the "Restarting kernel..." HTML) should work.

Member

takluyver commented Oct 6, 2014

It certainly looks like sticking self.reset(True) in the right part of restart_kernel (about where it adds the "Restarting kernel..." HTML) should work.

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

Merge pull request #1681 from ivanov/optionally-clear-on-restart
add qt config option to clear_on_kernel_restart.

This allows for restarting kernels without clearing the qtconsole,
while leaving a visible indication that the kernel has restarted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment