Make ip.reset() work the same in interactive or non-interactive code. #347

Closed
wants to merge 1 commit into
from

3 participants

@takluyver
IPython member

At present, ip.reset() sets execution_count to 0. If that's done interactively, it gets bumped up by 1 as it leaves run_cell, so the next prompt is 1. But in test code, we call ip.reset() outside of run_cell, so the next run_cell command executes as cell 0, leading to unexpected problems.

This makes ip.reset() set execution_count to 1, and set a boolean flag so that run_cell doesn't increment it immediately afterwards.

@minrk
IPython member

Thanks, I don't see any problem with this.

@fperez
IPython member

I'm not crazy about the solution, though I can't think of a cleaner one right now. What I don't like is that we're solving the problem by introducing a flag to sync various functions, and we now have to track that flag around. That means more special-case code and coupling between different parts of the code.

Ideally, we should think through the execution count logic so that these scenarios 'just work', rather than adjusting counters up and down based on flags...

In fact, I'm not even sure that reset should set the counter back to 0 (or 1): it gives a false sense of security because the kernel hasn't really been fully reset (extension code is still there, you may have trapped references in tracebacks, etc). So I'm actually inclined to flush all variables, but leave the counter where it was, as a sign to the user that they may have a mostly cleaned up namespace but they're still sitting on an old kernel.

The qt console can do a real kernel restart, and when we have a 2-process terminal one we'll be able to do the same, but for now I'm inclined to say that %reset should just leave that counter alone... Thoughts?

@minrk
IPython member

Since reset doesn't really reset, maybe it just shouldn't touch the execution counter.

@takluyver
IPython member

This is the code that gets called for a hard reset, i.e. we're aiming to completely nuke all references, and return the interpreter to a 'just started' state. If there's anything we're not clearing up in this, I consider it a bug. We also get a new history session, so it makes sense to me that the line numbers should go from 1 again. For a soft reset (%reset -s), we leave the execution_count untouched.

I know this isn't perfect, but it was the cleanest solution I could think of. The alternative would be to increment the execution_count before running the user code, and then have displayhook use execution_count-1, but that just feels untidy. And when I looked at the logic to produce the numbers in the prompts, I got lost in layers of unfamiliar code.

In the future, there may also be other functions we can call interactively which change the prompt number (I'm thinking if you want to do things between sessions), in which case the same problem will come up.

@minrk
IPython member

This actually points to a bigger problem:

In [1]: get_ipython().shell.run_cell('a=5')
ERROR! Session/line number was not unique in database. History logging moved to new session 648
Out[2]: False

In [3]: _1
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
/Users/minrk/ in ()
----> 1 _1

NameError: name '_1' is not defined

In [4]: _2
Out[4]: False

The execution count should probably be incremented at the top of run_cell, which should prevent this sort of thing.

@fperez
IPython member
@takluyver
IPython member

Min: any code that can call back into run_cell from interactive user code should use store_history=False for precisely that reason. I think I've tidied up all the obvious cases.

Fernando: OK, fair enough. I'll change %reset so it doesn't reset the execution count.

@takluyver takluyver closed this Apr 9, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment