Inoculate clearcmd extension into %reset functionality

[ivanov]

I have restored the functionality of this quarantined module, added new tests for it and polished it a bit, and made it available as part of the %reset magic.

Specifically, the %clear completions did not previously advertise the array argument option, and clear in did not clear _i, _ii, and _iii - both of these are now fixed.

I am removing the shadow_compress and shadow_nuke options, as they refer to functionality that does not appear to be used anywhere anymore.

update 2012-01-25
Ok, as was discussed bellow, I've moved incorporated this into the %reset magic.

[takluyver]

A few thoughts:

• Note that although we still have the ip.db storage for things like the %store magic, 'shadow history' no longer exists.
• I wonder if it makes sense to integrate this with the %reset magic, which clears the user namespace.
• I'm not quite sure why someone would want to clear input history. Output history might take a lot of memory, but input history shouldn't cause any problems.
• If it's in extensions, the test should go in extensions/tests.

[takluyver]

I suspect this probably disrupts some things that assume the input history from the session is consistent. I don't see any reason to clear input history, anyway.

[ivanov]

Well, the way i see it, there are no failing tests - so it's acceptable. If this will cause something to break, we should have a test that ensures that whatever this will cause to break isn't broken. See my 'elvis' example for a use case of this.

[ivanov]

Thanks for taking a look, Thomas.

Note that although we still have the ip.db storage for things like the %store magic, 'shadow history' no longer exists.

does this mean that shadow_nuke and shadow_compress don't really do anything anymore and should be eliminated?

I wonder if it makes sense to integrate this with the %reset magic, which clears the user namespace.

I was actually wondering why %clear used to live as an extension, and not as a built-in magic, maybe @fperez could chime in? It's different than reset, in that it doesn't do anything to the traditional python namespace - it's just dealing with the convenient things IPython provides, so I think keeping them different makes sense.

I'm not quite sure why someone would want to clear input history. Output history might take a lot of memory, but input history shouldn't cause any problems.

Well, depends on the use case. I can imagine someone pasting in 100s of MB of protein sequences as string literals and then just doing "...".find('elvis') (i.e. the output can be way smaller than the input for some use cases), so it's not for us to not allow that use case.

If it's in extensions, the test should go in extensions/tests.

I was just trying to revive this code to eliminate a "KnownFailure" in core/tests/test_magic.py - which is why I put the other tests into that same file, and not into extensions (though it used to live there). If someone chimes in that it would not be ok to include this as a regular magic, then I'll move all these tests out. Does that sound good?

[takluyver]

On 23 January 2012 23:34, Paul Ivanov reply@reply.github.com wrote:

does this mean that shadow_nuke and shadow_compress don't really do anything anymore and should be eliminated?

Yes, I think so.

Well, depends on the use case. I can imagine someone pasting in 100s of MB
of protein sequences as string literals and then just doing
"...".find('elvis') (i.e. the output can be way smaller than the input
for some use cases), so it's not for us to not allow that use case.

If I do:

In [1]: print "foo"
foo
In [2]: %clear in
In [3]: print "bar"

Then In[1] would point to 'print "bar"' (which should be In[3]). Magics
like %save and %macro are similarly affected.

I think we can 'clear' it without messing up the history numbers by doing
something like this:

self.history_manager.input_hist_parsed[:] = [''] * len(self.history_manager.input_hist_parsed)

[takluyver]

It seems GH doesn't like my markdown today. Hopefully it's clear enough.

[ivanov]

@takluyver - crystal - makes sense, adding your test now and reverting the ['']*len functionality

[ivanov]

ok, I've addressed the points Thomas brought up, here's what's left to figure out:

• If shadow_compress and shadow_nuke really don't affect anything, remove them
• either move tests to extensions/tests or make this a 'native' magic

[fperez]

• If shadow_compress and shadow_nuke really don't affect anything, remove them

I honestly don't remember what their purpose was when they were introduced. I suggest doing a careful grin code-wide to see if anything turns up that uses them. If it's only in quarantine or deathrow, we'll have a look. If there's nothing, nuke them. And if any significant use case turns up, we'll also have a more careful look.

• either move tests to extensions/tests or make this a 'native' magic

I'm fine with it being 'native', I think it was an extension also just for accidental historical reasons.

[ivanov]

I honestly don't remember what their purpose was when they were introduced. I suggest doing a careful grin code-wide to see if anything turns up that uses them. If it's only in quarantine or deathrow, we'll have a look.

this. quarantine/ipy_jot.py has the following lines:

  # which one works better?
  #all = ip.shadowhist.all()
  all = ip.shell.history_manager.input_hist_parsed

Other than that, shadowhist isn't referenced or used anywhere else in the entire code. I'll just nuke it an remove the comments from the excerpt above.

I'm fine with it being 'native', I think it was an extension also just for accidental historical reasons.

oops, turns out there's already a %clear magic defined in zmq/zmqshell.py - which clears the screen for qtconsole and console. Proposed solutions:

1. rename previously quarantined clear to clean
2. incorporate quarantined clear functionality into reset
3. combine the two magics called clear - currently, the quarantined version is a noop when called with no args - but this could clear the screen - whereas the screen clearing one does the same thing regardless of the arguments you give it. This is kind of nice for the single-process terminal side, since !clear and %clear would have the same meaning.
4. something else?

I like 3.

[takluyver]

Shadow history was a secondary history system that stored all your input, but in no particular order (like a persistent set), when the main history system only stored a limited number of entries. I combined the two when I rewrote history to use SQLite. At some point, we'll need some functionality to truncate the SQLite history, but it's low priority, and the code will be nothing like that for shadow history.

I prefer 2 (integrating it with reset). Both are about releasing data stored in memory. Running all of the 'clear' options here gets you close to what %reset does. It seems much more confusing to combine the purely aesthetic 'clear screen' with this 'clear data'.

[fperez]

On Tue, Jan 24, 2012 at 6:01 AM, Thomas
reply@reply.github.com
wrote:

I prefer 2 (integrating it with reset). Both are about releasing data stored in memory. Running all of the 'clear' options here gets you close to what %reset does. It seems much more confusing to combine the purely aesthetic 'clear screen' with this 'clear data'.

Agreed: they should be grouped by functionality (namespace vs screen)
rather than by name.

[ivanov]

overruled :) I've moved incorporated this into the %reset magic.

Also made the two reset magics reference one another via "See also".

[takluyver]

Great, thanks. I'll try to test this this evening.

[takluyver]

This seems to be working OK in Python 2.7 and 3.2.

One remaining point - could we make the argument case insensitive? Because the variables are In and Out, I tend to find myself doing %reset In and %reset Out. It would also fit with magics like %colors, which also involve choosing from a small set of possible arguments.

[ivanov]

Thanks for taking a look, Thomas. Yeah, I thought about making
the parameters case insensitive, too, but decided that it might
become confusing for people who might think you can reset any
named variable.

How about I'll just make it case insensitive, but we keep the
examples lower cased, as they are.

[takluyver]

Sounds good to me. @fperez , this is good to go as far as I'm concerned.

[fperez]

My only suggestion is to break up the test_reset_args into 4 separate functions, pretty much according to the blocks that are in there already. Since there's no real coupling between them, more granularity makes it easier to debug problems if something breaks, as each test is effectively a probe of a different part of the code. Note that you don't even need to create manually _ip, as that object is always available during the test suite for convenience.

[ivanov]

ok, makes sense, done.

[fperez]

Great, thanks. Running test suite now, if all passes will merge in a minute. Great job!

[juliantaylor]

I think this merge broke the sphinx doc generation:

File "/build/buildd/ipython-0.12+2291/docs/sphinxext/docscrape.py", line 208, in parse_item_name
    raise ValueError("%s is not a item name" % text)
ValueError: %reset_selective is not a item name

I have no idea of sphinx so I can't really debug this. But if I remove all the % from the reset* lines in IPython/core/magic.py it works again.

full log
https://launchpadlibrarian.net/91276130/buildlog_ubuntu-oneiric-i386.ipython_0.12%2B2291-0~5~oneiric1_FAILEDTOBUILD.txt.gz

[takluyver]

From the log, I note this is occurring in a function called
_parse_see_also(). Presumably the See also sections expect an object name.
I wonder if we can use reST syntax like:

%reset <magic_reset>

(GH will take the backticks as markdown - this should appear correctly by
email)

[takluyver]

See PR #1338