From 138b3a4aa3279ffaa570630a8e5355bf860f1bc6 2016-08-30 22:59:10 From: Matthias Bussonnier Date: 2016-08-30 22:59:10 Subject: [PATCH] Do not update the unders if they are user defined Closes jupyter/notebook#1628 (once there are tests). Still shift the internal reference to self._,__,___ but do not assign it in user ns. I would expect this to be a change of behavior and have the (slight) side effect that if any of the _, __, or __ are defined, then none of these are updated. We could update the logic to do the right think on leapfrog _, __ or __ if user-set, but is it worth it. If _ is in builtins, then _ will not update the history and _ will be the builtins, unless user set it explicitly in which case it takes precedence. Summary if user have set _ _ == value set by the user elif _ in builtins: _ == value in builtins._ else: _ == previous result. Note that this logic may fall down if the use set _ to a specific value, and have this save value returned while _ is also in builtin: In [1]: import gettext ; gettext.install('foo') In [2]: _ = 'Example' In [3]: _ Out[3]: 'Example' In [4]: _ Out[4]: 'Example' In [5]: _ Out[5]: > --- diff --git a/IPython/core/displayhook.py b/IPython/core/displayhook.py index 07d733e..83941e6 100644 --- a/IPython/core/displayhook.py +++ b/IPython/core/displayhook.py @@ -76,6 +76,9 @@ class DisplayHook(Configurable): # particular uses _, so we need to stay away from it. if '_' in builtin_mod.__dict__: try: + user_value = self.shell.user_ns['_'] + if user_value is not self._: + return del self.shell.user_ns['_'] except KeyError: pass @@ -195,13 +198,23 @@ class DisplayHook(Configurable): if result is not self.shell.user_ns['_oh']: if len(self.shell.user_ns['_oh']) >= self.cache_size and self.do_full_cache: self.cull_cache() - # Don't overwrite '_' and friends if '_' is in __builtin__ (otherwise - # we cause buggy behavior for things like gettext). - if '_' not in builtin_mod.__dict__: - self.___ = self.__ - self.__ = self._ - self._ = result + # Don't overwrite '_' and friends if '_' is in __builtin__ + # (otherwise we cause buggy behavior for things like gettext). and + # do not overwrite _, __ or ___ if one of these has been assigned + # by the user. + update_unders = True + for unders in ['_'*i for i in range(1,4)]: + if not unders in self.shell.user_ns: + continue + if getattr(self, unders) is not self.shell.user_ns.get(unders): + update_unders = False + + self.___ = self.__ + self.__ = self._ + self._ = result + + if ('_' not in builtin_mod.__dict__) and (update_unders): self.shell.push({'_':self._, '__':self.__, '___':self.___}, interactive=False) @@ -209,7 +222,7 @@ class DisplayHook(Configurable): # hackish access to top-level namespace to create _1,_2... dynamically to_main = {} if self.do_full_cache: - new_result = '_'+repr(self.prompt_count) + new_result = '_%s' % self.prompt_count to_main[new_result] = result self.shell.push(to_main, interactive=False) self.shell.user_ns['_oh'][self.prompt_count] = result diff --git a/IPython/core/tests/test_displayhook.py b/IPython/core/tests/test_displayhook.py index 3cca42a..67b99d5 100644 --- a/IPython/core/tests/test_displayhook.py +++ b/IPython/core/tests/test_displayhook.py @@ -26,3 +26,30 @@ def test_output_quiet(): with AssertNotPrints('2'): ip.run_cell('1+1;\n#commented_out_function()', store_history=True) + +def test_underscore_no_overrite_user(): + ip.run_cell('_ = 42', store_history=True) + ip.run_cell('1+1', store_history=True) + + with AssertPrints('42'): + ip.run_cell('print(_)', store_history=True) + + ip.run_cell('del _', store_history=True) + ip.run_cell('6+6', store_history=True) + with AssertPrints('12'): + ip.run_cell('_', store_history=True) + + +def test_underscore_no_overrite_builtins(): + ip.run_cell("import gettext ; gettext.install('foo')", store_history=True) + ip.run_cell('3+3', store_history=True) + + with AssertPrints('gettext'): + ip.run_cell('print(_)', store_history=True) + + ip.run_cell('_ = "userset"', store_history=True) + + with AssertPrints('userset'): + ip.run_cell('print(_)', store_history=True) + ip.run_cell('import builtins; del builtins._') +