Ticket #5939 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

Replace python interpreter with embedded ipython

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: critical Milestone: Release 3.0
Component: MantidPlot Keywords: IPython
Cc: Blocked By: #7950, #7980, #8006, #8018
Blocking: Tester: Martyn Gigg

Description (last modified by Nick Draper) (diff)

There is a pull request over at the ipython repository which implements an in-process kernel for ipython. See: https://github.com/ipython/ipython/pull/2397

The implication of this is we could have ipython in a window within MantidPlot - as the script interpreter is now. Currently we are not able to do this as in current ipython releases you have to use 2 processes and the back-end has to be in the MantidPlot process.

Once this pull request is merged and in an ipython release, we should consider whether we want to make this change. One thing to look out for is whether the implementation still requires matplotlib to be present as that would be a large dependency to add (this is the principle reason why the ipython console in MantidPlot is currently an optional add-in).

Change History

comment:1 Changed 8 years ago by Russell Taylor

  • Milestone changed from Release 2.3 to Unassigned

comment:2 Changed 8 years ago by Nick Draper

Consider this after bundling in the packages

comment:3 Changed 8 years ago by Nick Draper

  • Owner set to Martyn Gigg
  • Status changed from new to assigned
  • Milestone changed from Unassigned to Release 2.4

Russell may be able to pick some or most of this up.

comment:4 Changed 8 years ago by Gesner Passos

Last edited 8 years ago by Gesner Passos (previous) (diff)

comment:5 Changed 8 years ago by Gesner Passos

Last edited 8 years ago by Russell Taylor (previous) (diff)

comment:6 Changed 8 years ago by Russell Taylor

This has finally been merged into the ipython master (in a different pull request to the one in the description): https://github.com/ipython/ipython/pull/2724

This will put it in the next ipython release, though I don't know when that is due.

comment:7 Changed 8 years ago by Martyn Gigg

  • Milestone changed from Release 2.4 to Release 2.5

comment:8 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.5 to Release 2.6

Moved to r2.6 at the end of r2.5

comment:9 Changed 7 years ago by Martyn Gigg

  • Milestone changed from Release 2.6 to Release 2.7

Batch move to 2.7

comment:10 Changed 7 years ago by Nick Draper

  • Status changed from assigned to new

comment:11 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.7 to Backlog

comment:12 Changed 7 years ago by Nick Draper

  • Owner changed from Martyn Gigg to Russell Taylor
  • Description modified (diff)

comment:13 Changed 7 years ago by Russell Taylor

  • Priority changed from major to critical
  • Milestone changed from Backlog to Release 3.0

IPython v1.0, containing the necessary changes, is due out any day.

comment:14 Changed 7 years ago by Russell Taylor

  • Keywords Poker added

comment:15 Changed 7 years ago by Nick Draper

  • Keywords Poker removed

Poker Estimate: 18d

comment:16 Changed 7 years ago by Russell Taylor

  • Status changed from new to inprogress

comment:17 Changed 7 years ago by Russell Taylor

Re #5939. Remove existing ipython console launch mechanism.

Changeset: 037b59219fc8569d55c19d3c8e16581c7e5ca428

comment:18 Changed 7 years ago by Russell Taylor

Re #5939. Remove usage of existing script interpreter.

Have left the QTextEdit (that was previously only used temporarily) as a placeholder.

Changeset: c2058d49f1fb60ccfc1910d30d59d2df704b2cdf

comment:19 Changed 7 years ago by Russell Taylor

Re #5939. Remove existing script interpreter class.

Changeset: f4fa114614e24c1242b8855815ccca2c9b88b69b

comment:20 Changed 7 years ago by Russell Taylor

Re #5939. Add an IPython console as the script interpreter.

For now, requires IPython 1.0 to be installed locally.

Changeset: 1497115d4edcd59a3e25f6ed1bf56216d6dfafe7

comment:21 Changed 7 years ago by Russell Taylor

It works (on RHEL6 at least), but there are two notable issues:

  • It locks up the GUI until a command returns (our existing external ipython console does this as well; our current script interpreter of course does not).
  • It will require us to change to the PyQt v2 API for at least QString & QVariant. This will break things, and of course with python you quite possibly don't know it's broken until you try to execute the broken code.
Last edited 7 years ago by Russell Taylor (previous) (diff)

comment:22 Changed 7 years ago by Russell Taylor

Turns out it doesn't entirely work. The line kernel_manager.start_kernel() in mantid_ipython_widget.py destroys the import of the mantid modules into the 'F3' python window.

Other news:

  • The Mac behaves the same as linux, once ipython, pygments, pyzmq & readline are installed (matplotlib, which was the pain before, is not required).
  • On Windows we need to upgrade pyzmq and the libraries in the newer versions suffer from the same problem we had before, which is detailed here.

Other niggles:

  • Related to the locking of the GUI, you can't interrupt a long-running command.
  • The qt console appears to lack the "--no-banner" and "--nosep" options that the regular ipython terminal have to skip the start-up text and not put a blank line between every input.

comment:23 Changed 7 years ago by Russell Taylor

Although I'm not terribly keen on the notion of forking IPython, I do find that the following change to the IPython 1.1 code solves the locking issue:

+++ b/IPython/core/interactiveshell.py
@@ -29,6 +29,7 @@
 import sys
 import tempfile
 import types
+import threading
 from io import open as io_open
 
 from IPython.config.configurable import SingletonConfigurable
@@ -78,6 +79,8 @@
 from IPython.utils.warn import warn, error
 import IPython.core.hooks
 
+from PyQt4 import QtGui
+
 #-----------------------------------------------------------------------------
 # Globals
 #-----------------------------------------------------------------------------
@@ -89,6 +92,9 @@
 # Utilities
 #-----------------------------------------------------------------------------
 
+def runner(code_obj, global_ns, user_ns):
+    exec code_obj in global_ns, user_ns
+
 @undoc
 def softspace(file, newvalue):
     """Copied from code.py, to remove the dependency"""
@@ -2824,7 +2830,11 @@ def run_code(self, code_obj):
             try:
                 self.hooks.pre_run_code_hook()
                 #rprint('Running code', repr(code_obj)) # dbg
-                exec code_obj in self.user_global_ns, self.user_ns
+                #exec code_obj in self.user_global_ns, self.user_ns
+                t = threading.Thread(target=runner, args=(code_obj, self.user_g
+                t.start()
+                while t.is_alive():
+                    QtGui.QApplication.processEvents()
             finally:
                 # Reset our crash handler in place
                 sys.excepthook = old_excepthook

N.B. This way does spoil the nice IPython messages in case of errors (because the exception comes out of the separate thread so isn't caught and dealt with in run_code). This is certainly something that could be dealt with in some way.

comment:24 Changed 7 years ago by Russell Taylor

It seems that with IPython 1.1, all the Mantid modules are available in the F3 window. Autocomplete doesn't work though. Still, that's an improvement. It might be related to how I specifically import mantidplot & mantid.simpleapi in mantid_ipython_widget.py rather than executing mantidplotrc.py (I see errors that 'module' object has no attribute '_ScopeInspector_GetFunctionAttributes').

comment:25 Changed 7 years ago by Russell Taylor

Re #5939. Run mantidplotrc on IPython startup.

This is in place of importing the individual mantid & mantidplot modules and means that code completion in the 'F3 script window' still works.

Changeset: e50f99ac739defa0ac019847dbe4bfe26e16bba3

comment:26 Changed 7 years ago by Russell Taylor

Re #5939. Better way of constructing the path to mantidplotrc.py

Changeset: 57233c39762c5561da67f98de59662ac581abe84

comment:27 Changed 7 years ago by Russell Taylor

Re #5939. Make sure any local IPython is picked up first.

Before a system installed one.

Changeset: 337347cbff36a4bdac217844fee7d02aadb6ebf2

comment:28 Changed 7 years ago by Russell Taylor

I've tested this, using the change to IPython in comment:23, on RHEL6, OS X 10.8 & Windows and in all cases things appear to be working very nicely.

On Windows, I had to mess with the manifest of libzmq as follows to make it importable:

mt.exe -inputresource:python27.dll;#2 -outputresource:Lib\site-packages\zmq\libzmq.pyd;#2

This is after downloading the lastest version. Thankfully that was the only one I had to change (it was many more last time).

comment:29 Changed 7 years ago by Russell Taylor

Re #5939. Add a mechanism to avoid blocking MantidPlot while running.

We replace the IPython method that actually executes the entered code with one that then calls the original one on a separate thread so that we can continue to process GUI events while the command runs.

Changeset: acb618cdd6a74e5169fd90535bb15fead60b096c

comment:30 Changed 7 years ago by Russell Taylor

  • Blocked By 7950 added

comment:31 Changed 7 years ago by Russell Taylor

  • Blocked By 7980 added

comment:32 Changed 7 years ago by Russell Taylor

  • Blocked By 7981 added

comment:33 Changed 7 years ago by Russell Taylor

  • Blocked By 8006 added

comment:34 Changed 7 years ago by Russell Taylor

  • Blocked By 8018 added

comment:35 Changed 7 years ago by Russell Taylor

Re #5939. Remove existing ipython console launch mechanism.

Changeset: f12945978c0479910dbf202ebb891bad50bb127a

comment:36 Changed 7 years ago by Russell Taylor

Re #5939. Remove usage of existing script interpreter.

Have left the QTextEdit (that was previously only used temporarily) as a placeholder.

Changeset: 5ee3ec30e00a34d84ddc89d5af711c4da43f07b5

comment:37 Changed 7 years ago by Russell Taylor

Re #5939. Remove existing script interpreter class.

Changeset: 10c88e75faf1e98585d60065a0f8a0fdff340dd5

comment:38 Changed 7 years ago by Russell Taylor

Re #5939. Add an IPython console as the script interpreter.

Changeset: f73e29671cd30402475c62262da1f39c2cbfee63

comment:39 Changed 7 years ago by Russell Taylor

Re #5939. Run mantidplotrc on IPython startup.

This is in place of importing the individual mantid & mantidplot modules and means that code completion in the 'F3 script window' still works.

Changeset: abcea42457e9020f92d9399d1023e1744ff713aa

comment:40 Changed 7 years ago by Russell Taylor

Re #5939. Better way of constructing the path to mantidplotrc.py

Changeset: e5123796c09ffb18d7fc0986383080d11fa78492

comment:41 Changed 7 years ago by Russell Taylor

Re #5939. Make sure any local IPython is picked up first.

Before a system installed one.

Changeset: 4923824b0090050c4f1919c53f66913384c9219b

comment:42 Changed 7 years ago by Russell Taylor

Re #5939. Add a mechanism to avoid blocking MantidPlot while running.

We replace the IPython method that actually executes the entered code with one that then calls the original one on a separate thread so that we can continue to process GUI events while the command runs.

Changeset: a92a0fb5c6efd755481204d21e9bd555811d3ff4

comment:43 Changed 7 years ago by Russell Taylor

  • Keywords IPython added

comment:44 Changed 7 years ago by Russell Taylor

Re #5939. Add IPython 1.1 to list of dependent rpms.

Changeset: f8f57896c4ba3966edb853d27e560f4454507c87

comment:45 Changed 7 years ago by Russell Taylor

Re #5939. Try to fix test that's failing in some places.

Changeset: 55f7f256ad67871aa40968e47ad4650db71b9da6

comment:46 Changed 7 years ago by Russell Taylor

Re #5939. English corrections in docstring.

Changeset: 4f17cd9e5034eb6ce98deb6e0090f5f3d2953127

comment:47 Changed 7 years ago by Russell Taylor

  • Blocked By 7981 removed

Removing the ticket for creating Ubuntu packages as this needs to get into master.

comment:48 Changed 7 years ago by Russell Taylor

  • Status changed from inprogress to verify
  • Resolution set to fixed

Tester: Branch is feature/5939_ipython_interpreter

If you are on RHEL6, be sure to yum install python-ipython if you haven't already. If on Ubuntu, you'll need to easy_install ipython. For Mac & Windows it's in Third_Party and will be copied to the right place.

To test, just make sure that the IPython console starts up correctly in MantidPlot and have a bit of a play to check that things are working. Check also that the 'other' (F3) python window works as before.

Hopefully, this will get a good shake-down by people using it in the run-up to the release. This is why I've decided I can wait no longer for the proper Ubuntu packages to be there - I really wanted to get this in a long time before now. The implication is that, until the deb packages are there, anyone using the nightly build on Ubuntu (and I expect this is between zero and very few people) will have to easy_install the latest IPython or go without the script interpreter.

Note that there's a known issue with tab-completion not working very well for some MantidPlot objects (see #4311).

comment:49 Changed 7 years ago by Russell Taylor

  • Status changed from verify to reopened
  • Resolution fixed deleted

It seems that we need to solve an issue with loading the MSVC runtime on Windows.

comment:50 Changed 7 years ago by Martyn Gigg

  • Blocked By 8179 added

comment:51 Changed 7 years ago by Martyn Gigg

  • Blocked By 8179 removed

(In #8179) All of the changes here happened in the third party repositories so there is no actual code to merge.

Tester: Requires Windows with a copy of something that has MSVCR90.dll in the PATH. I suggest an ISIS machine that has exceed installed. This is how the problem was originally found. Make sure you run fetch_Third_Party.bat to bring down the latest changes and then do

git fetch -p git checkout master git reset --hard origin/master

in your main code directory. Now check that MantidPlot starts without error and that you can use the IPython interpreter (F4).

comment:52 Changed 7 years ago by Russell Taylor

  • Status changed from reopened to verify
  • Resolution set to fixed

#8179 should have resolved the windows issue. Still no Ubuntu packages yet unfortunately.

Last edited 7 years ago by Russell Taylor (previous) (diff)

comment:53 Changed 7 years ago by Martyn Gigg

  • Status changed from verify to verifying
  • Tester set to Martyn Gigg

comment:54 Changed 7 years ago by Martyn Gigg

Tried on all 3 platforms and things look good.

comment:55 Changed 7 years ago by Martyn Gigg

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/5939_ipython_interpreter'

Full changeset: e8ad18f9b7da761024eaaa35a76c947525dc82f4

comment:56 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6785

Note: See TracTickets for help on using tickets.