Skip to content
This repository

Make pylab import all configurable #622

Merged
merged 4 commits into from almost 3 years ago

4 participants

Jens H Nielsen Fernando Perez Min RK Thomas Kluyver
Jens H Nielsen

New version of pull request #551 This should of cause not be merged until after 0.11
I have added a test and changed the magic function to use the import_all setting.

I few issues remain:

I don't like the way that the magic function works as the default value True is hard coded when get_ipython().TerminalIPythonApp.pylab_import_all is not set. Is there a simple way to get the default value here?

The test uses the magic function. It results in 2 warnings because of problems with the gtk hook and because matplotlib.use. It would be better with a test that creates a new instance of IPython only for the test. I tried creating a new instance with code similar to the TerminalIPythonApp launch_new_instance. However this seems to lock the test framework. Any hints for making a better test are welcome.

Fernando Perez
Owner

This looks good, but I have a couple of comments:

  • Your test code can be simplified by replacing
        ip.magic('pylab')
        ip.run_cell('a = dir()')
        ipdir = ip.user_ns['a']
        has_plot = False
        for ipd in ipdir:
            if ipd == 'plot':
                has_plot = True
        self.assertTrue(has_plot)
    

with

        ip.magic('pylab')
        self.assertTrue('plot' in ip.user_ns)

which is clearer, faster and fully equivalent.

  • The flag should really be added to core/shellapp.py, where incidentally all the pylab flags should be moved (right now we have duplicated them in both the terminal and the qt console). By doing this, we'll remove the duplication and your new option will work for both the terminal and the qt console, which right now it doesn't.

  • @minrk, could you help me out with the following: which is the object that has the fully merged configuration data, that includes the overlaying of the various defaults with the stuff in get_ipython().config? I'm still a bit fuzzy on those details.

In summary, I think if we work a bit on this one, not only will we get the import_all situation fixed, but more generally where we handle pylab for all ipython shells.

Min RK
Owner
Fernando Perez
Owner

Got it, thanks @minrk. So indeed, @jenshnielsen, the way to go is to clean things out and put the pylab flags only in the core.shellapp.InteractiveShellApp object, and then both the terminal and the Qt console will get them.

From looking around, I get the feeling that we need to do a little cleanup of all the classes, but you don't need to worry about that quite now.

Jens H Nielsen

I changed the test and moved pylab_import_all to core.shellapp.InteractiveShellApp
However I did not move the pylab option. The problem is that it that the pylab flag has a inline option
in the qtconsole. The terminal app does of cause not have this option.

The test is still potentially problematic because pylab is activated in the testing env. and I guess that this
can give problems in the following tests. Maybe one could implement a none option for pylab that only does the
import but doesn't activate the gui integration.

Fernando Perez
Owner

Thanks much. I won't have time to finish this in the next couple of days, but hopefully on Wednesday at the mini-sprint prior to euroscipy I can finish it up.

IPython/core/magic.py
... ... @@ -3454,7 +3454,14 @@ Defaulting color scheme to 'NoColor'"""
3454 3454 Backend in use: Qt4Agg
3455 3455 For more information, type 'help(pylab)'.
3456 3456 """
3457   - self.shell.enable_pylab(s)
  3457 +
  3458 + try:
  3459 + ip = get_ipython()
9
Thomas Kluyver Collaborator

I think this should be the same object as self.shell in this context, so you needn't use get_ipython.

Yes thats simpler, thanks. The try except AttributeError: construction
still seems to be necessary as pylab_import_all has doesn't have any
value if it hasn't been customized in a config file. Is there a simple way to get the default value
so that true doesn't have to be hard coded here?.

Thomas Kluyver Collaborator

Hmm. I think you're currently using a reference to the config itself. What we'd need is a reference to the application object itself, but I don't know if it's possible to get one from the shell object.

@fperez, @minrk : Is there a way to get a reference to the application from inside a magic method?

Min RK Owner
minrk added a note

current Application can always be retrieved with:

from IPython.config.application import Application
app = Application.instance()

That works except in the test. The test returns an instance of the basic application without pylab_import_all.
In the IPython shell it returns a TerminalIPythonApp instance that has pylab_import_all
defined. I worked around this by falling back to self.shell.config.TerminalIPythonApp.pylab_import_all
that works in the test. But is not defined in the IPython shell when it hasn't been customized.

Thomas Kluyver Collaborator

Generally I prefer to avoid having code in the core just for compatibility with the test suite. Two possible ways round it:

  • The test calls Application.instance() before the code, and sets a pylab_import_all attribute.
  • The test runs a separate IPython process and gets it to print some output indicating whether or not names from pylab have been imported.

Thanks for this work - I hope it doesn't feel like a wild goose chase.

Min RK Owner
minrk added a note

Yes, thanks for working on this. The configuration stuff is definitely going through some growing pains, as it's brand new and not the most convenient for everything. The test suite should presumably start an Application, so its environment is more like that seen by a real IPython, but that's out of scope for this.

For now, I would create the Application in your tests, rather than writing to the config object itself. Add:

app = Application.instance()
app.pylab_import_all = True/False

in your tests,

and then check in the magic should not go directly to the config object, but straight to a default in the application doesn't have the right attribute:

        if Application.initialized():
            app = Application.instance()
            try:
                import_all_status = app.pylab_import_all
            except AttributeError:
                import_all_status = True # (or False, whatever the default is)
        else:
            import_all_status = True

That way you can have a test that includes the case where the Application does not even have the trait.

Thanks I have implemented this exactly as you suggested and it works.
I am still a bit worried about the test. I think we need a way to avoid that the pylab magic activates the gui
in order to avoid problems in subsequent tests.

Min RK Owner
minrk added a note

If you are concerned about polluting the environment, @takluyver is right, performing the test via an IPython subprocess is safer. There are various irunner tests already in the test suite that give examples of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Jens H Nielsen

New version of the test using now irunner. I copied and modified the test_irunner.py a bit.
It was needed to change it to use a regex because pylab prints text that depend on the backend.

Note that one of the tests fails when running it from virtualenv. The virualenv irunner seems to use the system wide
ipython install not the one in the virtualenv. (Verified by inspecting get_ipython parameters from the virualenv)

IPython/lib/tests/test_irunner_pylab_magic.py
((74 lines not shown))
  74 +In \[2\]: app = Application\.instance\(\)
  75 +In \[3\]: app\.pylab_import_all = True
  76 +In \[4\]: pylab
  77 +^Welcome to pylab, a matplotlib-based Python environment
  78 +For more information, type 'help\(pylab\)'\.
  79 +In \[5\]: ip=get_ipython\(\)
  80 +In \[6\]: \'plot\' in ip\.user_ns
  81 +Out\[6\]: True
  82 +"""
  83 + runner = irunner.IPythonRunner(out=self.out)
  84 + self._test_runner(runner,source,output)
  85 +
  86 + @decorators.skipif_not_matplotlib
  87 + def test_pylab_import_all_disabled(self):
  88 + "Verify that plot is not available when pylab_import_all = False"
  89 + runner = irunner.PythonRunner(out=self.out)
3
Min RK Owner
minrk added a note

Is this line supposed to do anything?

Min RK Owner
minrk added a note

There's a duplicate line in the other test. Presumably leftovers from the copy/paste.

Thanks. I have removed the useless line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Min RK
Owner

The real code looks great to me.

I'm not sure why the irunner would not spawn the same IPython it's being run from - it does inherit the path/env as I understand, but virtualenv can fail at such things. It is possible that the path/env does not get set up or inherited properly, but this has not been my experience.

Jens H Nielsen

I submitted a new issue #779 for the irunner problem. It is independent of both this pull request and the general testing
env.

Min RK
Owner

Okay, I think this looks good. Thanks for opening the virtualenv-test Issue. It now conflicts with a big recent py3compat merge, so if you rebase again on master, I will do the merge. If you'd rather not do that, I can to a rebased merge myself.

Jens H Nielsen

Thanks. I have already another local branch with the changes squashed into 4 logical commits and
rebased on master from yesterday. I will push that on top of this one later today.

Jens H Nielsen

Rebased

Min RK
Owner

Thanks! Looks good to me, passes tests.

Merged.

Min RK minrk merged commit 0fc7291 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
13 IPython/core/magic.py
@@ -61,6 +61,7 @@
61 61 from IPython.utils.timing import clock, clock2
62 62 from IPython.utils.warn import warn, error
63 63 from IPython.utils.ipstruct import Struct
  64 +from IPython.config.application import Application
64 65
65 66 #-----------------------------------------------------------------------------
66 67 # Utility functions
@@ -3442,7 +3443,17 @@ def magic_pylab(self, s):
3442 3443 Backend in use: Qt4Agg
3443 3444 For more information, type 'help(pylab)'.
3444 3445 """
3445   - self.shell.enable_pylab(s)
  3446 +
  3447 + if Application.initialized():
  3448 + app = Application.instance()
  3449 + try:
  3450 + import_all_status = app.pylab_import_all
  3451 + except AttributeError:
  3452 + import_all_status = True
  3453 + else:
  3454 + import_all_status = True
  3455 +
  3456 + self.shell.enable_pylab(s,import_all=import_all_status)
3446 3457
3447 3458 def magic_tb(self, s):
3448 3459 """Print the last traceback with the currently active exception mode.
6 IPython/core/shellapp.py
@@ -29,7 +29,7 @@
29 29 from IPython.config.configurable import Configurable
30 30 from IPython.config.loader import Config
31 31 from IPython.utils.path import filefind
32   -from IPython.utils.traitlets import Unicode, Instance, List
  32 +from IPython.utils.traitlets import Unicode, Instance, List, Bool
33 33
34 34 #-----------------------------------------------------------------------------
35 35 # Aliases and Flags
@@ -133,6 +133,10 @@ def _extra_extension_changed(self, name, old, new):
133 133 code_to_run = Unicode('', config=True,
134 134 help="Execute the given command string."
135 135 )
  136 + pylab_import_all = Bool(True, config=True,
  137 + help="""If true, an 'import *' is done from numpy and pylab,
  138 + when using pylab"""
  139 + )
136 140 shell = Instance('IPython.core.interactiveshell.InteractiveShellABC')
137 141
138 142 def init_shell(self):
5 IPython/frontend/terminal/ipapp.py
@@ -346,7 +346,10 @@ def init_gui_pylab(self):
346 346 try:
347 347 self.log.info("Enabling GUI event loop integration, "
348 348 "toolkit=%s, pylab=%s" % (gui, self.pylab) )
349   - activate(gui)
  349 + if self.pylab:
  350 + activate(gui, import_all=self.pylab_import_all)
  351 + else:
  352 + activate(gui)
350 353 except:
351 354 self.log.warn("Error in enabling GUI event loop integration:")
352 355 self.shell.showtraceback()
108 IPython/lib/tests/test_irunner_pylab_magic.py
... ... @@ -0,0 +1,108 @@
  1 +"""Test suite for pylab_import_all magic
  2 +Modified from the irunner module but using regex.
  3 +"""
  4 +
  5 +# Global to make tests extra verbose and help debugging
  6 +VERBOSE = True
  7 +
  8 +# stdlib imports
  9 +import cStringIO as StringIO
  10 +import sys
  11 +import unittest
  12 +import re
  13 +
  14 +# IPython imports
  15 +from IPython.lib import irunner
  16 +from IPython.testing import decorators
  17 +
  18 +# Testing code begins
  19 +class RunnerTestCase(unittest.TestCase):
  20 +
  21 + def setUp(self):
  22 + self.out = StringIO.StringIO()
  23 + #self.out = sys.stdout
  24 +
  25 + def _test_runner(self,runner,source,output):
  26 + """Test that a given runner's input/output match."""
  27 +
  28 + runner.run_source(source)
  29 + out = self.out.getvalue()
  30 + #out = ''
  31 + # this output contains nasty \r\n lineends, and the initial ipython
  32 + # banner. clean it up for comparison, removing lines of whitespace
  33 + output_l = [l for l in output.splitlines() if l and not l.isspace()]
  34 + out_l = [l for l in out.splitlines() if l and not l.isspace()]
  35 + mismatch = 0
  36 + if len(output_l) != len(out_l):
  37 + message = ("Mismatch in number of lines\n\n"
  38 + "Expected:\n"
  39 + "~~~~~~~~~\n"
  40 + "%s\n\n"
  41 + "Got:\n"
  42 + "~~~~~~~~~\n"
  43 + "%s"
  44 + ) % ("\n".join(output_l), "\n".join(out_l))
  45 + self.fail(message)
  46 + for n in range(len(output_l)):
  47 + # Do a line-by-line comparison
  48 + ol1 = output_l[n].strip()
  49 + ol2 = out_l[n].strip()
  50 + if not re.match(ol1,ol2):
  51 + mismatch += 1
  52 + if VERBOSE:
  53 + print '<<< line %s does not match:' % n
  54 + print repr(ol1)
  55 + print repr(ol2)
  56 + print '>>>'
  57 + self.assert_(mismatch==0,'Number of mismatched lines: %s' %
  58 + mismatch)
  59 +
  60 + @decorators.skipif_not_matplotlib
  61 + def test_pylab_import_all_enabled(self):
  62 + "Verify that plot is available when pylab_import_all = True"
  63 + source = """
  64 +from IPython.config.application import Application
  65 +app = Application.instance()
  66 +app.pylab_import_all = True
  67 +pylab
  68 +ip=get_ipython()
  69 +'plot' in ip.user_ns
  70 + """
  71 + output = """
  72 +In \[1\]: from IPython\.config\.application import Application
  73 +In \[2\]: app = Application\.instance\(\)
  74 +In \[3\]: app\.pylab_import_all = True
  75 +In \[4\]: pylab
  76 +^Welcome to pylab, a matplotlib-based Python environment
  77 +For more information, type 'help\(pylab\)'\.
  78 +In \[5\]: ip=get_ipython\(\)
  79 +In \[6\]: \'plot\' in ip\.user_ns
  80 +Out\[6\]: True
  81 +"""
  82 + runner = irunner.IPythonRunner(out=self.out)
  83 + self._test_runner(runner,source,output)
  84 +
  85 + @decorators.skipif_not_matplotlib
  86 + def test_pylab_import_all_disabled(self):
  87 + "Verify that plot is not available when pylab_import_all = False"
  88 + source = """
  89 +from IPython.config.application import Application
  90 +app = Application.instance()
  91 +app.pylab_import_all = False
  92 +pylab
  93 +ip=get_ipython()
  94 +'plot' in ip.user_ns
  95 + """
  96 + output = """
  97 +In \[1\]: from IPython\.config\.application import Application
  98 +In \[2\]: app = Application\.instance\(\)
  99 +In \[3\]: app\.pylab_import_all = False
  100 +In \[4\]: pylab
  101 +^Welcome to pylab, a matplotlib-based Python environment
  102 +For more information, type 'help\(pylab\)'\.
  103 +In \[5\]: ip=get_ipython\(\)
  104 +In \[6\]: \'plot\' in ip\.user_ns
  105 +Out\[6\]: False
  106 +"""
  107 + runner = irunner.IPythonRunner(out=self.out)
  108 + self._test_runner(runner,source,output)
2  IPython/testing/decorators.py
@@ -313,6 +313,8 @@ def module_not_available(module):
313 313 # Other skip decorators
314 314 skipif_not_numpy = skipif(module_not_available('numpy'),"This test requires numpy")
315 315
  316 +skipif_not_matplotlib = skipif(module_not_available('matplotlib'),"This test requires matplotlib")
  317 +
316 318 skipif_not_sympy = skipif(module_not_available('sympy'),"This test requires sympy")
317 319
318 320 skip_known_failure = knownfailureif(True,'This test is known to fail')
4 IPython/zmq/ipkernel.py
@@ -623,10 +623,6 @@ class IPKernelApp(KernelApp, InteractiveShellApp):
623 623 selecting a particular matplotlib backend and loop integration.
624 624 """
625 625 )
626   - pylab_import_all = Bool(True, config=True,
627   - help="""If true, an 'import *' is done from numpy and pylab,
628   - when using pylab"""
629   - )
630 626 def initialize(self, argv=None):
631 627 super(IPKernelApp, self).initialize(argv)
632 628 self.init_shell()

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.