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

lines ending with semicolon should not go to cache #307

Closed
ivanov opened this Issue Mar 25, 2011 · 8 comments

Comments

Projects
None yet
3 participants
@ivanov
Member

ivanov commented Mar 25, 2011

on ipython 0.10.2 and prior, putting a semicolon at the end of a matplotlib object causes it to not be cached.
i.e.

In [1]: plt.figure();
In [2]:

However, on trunk

In [1]: plt.figure();
Out[1]: <matplotlib.figure.Figure at 0x95cd34c>
In [2]:
@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov Mar 25, 2011

Member

test for this bug is in trunk as of 94644f8. You will find it in the file IPython/core/tests/test_interactiveshell.py - please remove the skip_known_failure decorator to work on this bug.

Member

ivanov commented Mar 25, 2011

test for this bug is in trunk as of 94644f8. You will find it in the file IPython/core/tests/test_interactiveshell.py - please remove the skip_known_failure decorator to work on this bug.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Apr 7, 2011

Member

Will it be sufficient just to test removecomments(source).strip().endswith(";")? As far as I know, the semicolon doesn't trigger anything different in how Python parses and runs the code, so I think we have to do it ourselves.

Member

takluyver commented Apr 7, 2011

Will it be sufficient just to test removecomments(source).strip().endswith(";")? As far as I know, the semicolon doesn't trigger anything different in how Python parses and runs the code, so I think we have to do it ourselves.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Apr 8, 2011

Member

The bug is in displayhook, which has a quiet() method that checks for cells ending with ';\n', but the trailing newline is no longer there, so it always returns False.

Pushing fix / unskipping test presently...

Member

minrk commented Apr 8, 2011

The bug is in displayhook, which has a quiet() method that checks for cells ending with ';\n', but the trailing newline is no longer there, so it always returns False.

Pushing fix / unskipping test presently...

@minrk minrk closed this in 1f0e9da Apr 8, 2011

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Apr 9, 2011

Member

I'm getting this test failing on master now.

Member

takluyver commented Apr 9, 2011

I'm getting this test failing on master now.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Apr 9, 2011

Member

Ah, I've found it. When you call ip.reset(), the execution counter is set to zero. If you do that inside IPython, run_cell increments the execution counter as it finishes, so the next prompt is number 1. But when you do it in the test suite, you do ip.reset() without run_cell(), so looking up the item in the history is out by one.

Of course, this means it passes depending on what tests are run before it.

Member

takluyver commented Apr 9, 2011

Ah, I've found it. When you call ip.reset(), the execution counter is set to zero. If you do that inside IPython, run_cell increments the execution counter as it finishes, so the next prompt is number 1. But when you do it in the test suite, you do ip.reset() without run_cell(), so looking up the item in the history is out by one.

Of course, this means it passes depending on what tests are run before it.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Apr 9, 2011

Member

that's interesting, I was never able to cause it to fail on my machine.

This is now a bug in the history manager, right? If ip.reset() from non-interactive code results in an inconsistent state?

Member

minrk commented Apr 9, 2011

that's interesting, I was never able to cause it to fail on my machine.

This is now a bug in the history manager, right? If ip.reset() from non-interactive code results in an inconsistent state?

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Apr 9, 2011

Member

iptest IPython.core fails for me. It's a bug in run_cell and reset, and I'm a bit surprised we haven't noticed it before. I think ip.reset() needs to leave execution_count as 1, and set a flag so that run_cell then doesn't increment it again.

Member

takluyver commented Apr 9, 2011

iptest IPython.core fails for me. It's a bug in run_cell and reset, and I'm a bit surprised we haven't noticed it before. I think ip.reset() needs to leave execution_count as 1, and set a flag so that run_cell then doesn't increment it again.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Apr 9, 2011

Member

Made a pull request, #347. This test passes for me with that change.

Member

takluyver commented Apr 9, 2011

Made a pull request, #347. This test passes for me with that change.

markvoorhies pushed a commit to markvoorhies/ipython that referenced this issue Apr 21, 2011

ivanov added a commit to ivanov/ipython that referenced this issue Jan 25, 2012

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

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

fix displayhook.quiet() check
enable @ivanov's test, as it now passes

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