Opened 10 years ago
Last modified 6 years ago
#12719 closed enhancement
Upgrade to IPython 0.13 — at Version 74
Reported by: | kini | Owned by: | tbd |
---|---|---|---|
Priority: | critical | Milestone: | sage-5.7 |
Component: | packages: standard | Keywords: | sd40.5 |
Cc: | iandrus, vbraun, jason, AlexanderDreyer, robertwb | Merged in: | |
Authors: | Mike Hansen, Volker Braun, Jason Grout | Reviewers: | Volker Braun, Mike Hansen, Jason Grout |
Report Upstream: | N/A | Work issues: | ?? doesn't show code |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
We need to upgrade IPython to the shiny new thing. Well, not really - the shiny new thing was 0.11, and 0.13 is now stable. It's been mostly rewritten and is now a lot more modular, as I understand it. Definitely worth using in Sage.
Note: To use the QT console (sage -ipython qtconsole
), you need qt/qt-devel in your base OS install, the optional zeromq/pyzmq spkgs from #12843, and the optional sip/PyQt spkgs from #13022.
Install the IPython 0.13 spkg at http://sage.math.washington.edu/home/jason/ipython-0.13.spkg
apply:
To the scripts repository:
To the root repository:
To the sage library:
- trac_12719-5.1beta1.patch
- trac_12719-move_to_preparser.patch
- trac_12719_ipython_fixes.patch
- trac_12719_dedent.patch
- trac_12719-crash.patch
- trac_12719-cellmagics013.patch
cd SAGE_ROOT cd local/bin hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-scripts.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_remove_sage_gdb_ipython.patch cd ../../ hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_ROOT_configuration_files.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-ipythondir013.patch cd devel/sage hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-5.1beta1.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-move_to_preparser.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_ipython_fixes.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_dedent.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-crash.patch hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-cellmagics013.patch cd ../../ ./sage -br
Change History (79)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Would be nice but we need some porting, here is what typically happen if you try to use ipython-0.12 with sage (we had several reports in sage-on-gentoo before I pinned down the version used by sage):
$ sage ---------------------------------------------------------------------- | Sage Version 4.7.2, Release Date: 2011-10-29 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/bin/sage-ipython", line 19, in <module> from IPython.CrashHandler import CrashHandler ImportError: No module named CrashHandler
comment:3 Changed 10 years ago by
Yes, obviously :) I'm just making this ticket since it is absolutely must be an eventual goal for us and nobody else has made a ticket yet, even though IPython 0.12 has been out for about three months.
comment:4 Changed 10 years ago by
To be honest Fernando Perez and I talked about it on the mailing list at the time of ipython 0.11 and I said I would tell him what happens when we use ipython 0.11+ with sage because he is interested. For various reason I haven't been able to go back to it. But some sage-on-gentoo users eventually got a taste of it when it finally landed in the portage tree.
So first hurdle: do something about the crash handling.
comment:5 Changed 10 years ago by
- Cc iandrus added
comment:6 Changed 10 years ago by
I'm working on this now, and have made quite a bit of progress. As an added bonus, making the Sage customizations to IPython is quite a bit cleaner. I should hopefully be able to clean out the mess in sage/misc/preparser_ipython.py and sage/misc/interpreter.py .
comment:7 Changed 10 years ago by
This will fix #4413.
comment:8 Changed 10 years ago by
Great, I'm glad you're on the case! :)
comment:9 Changed 10 years ago by
- Cc vbraun added
comment:10 Changed 10 years ago by
What about zmq/pyzmq? It would really add to the functionality and I'm interested in the parallel/cluster support. But, strictly speaking, its not necessary to run ipython. Its also used in the new notebook so maybe its time for it to become a standard package?
comment:11 Changed 10 years ago by
- Cc jason added
CCing Jason as he expressed interest on sage-devel.
comment:12 Changed 10 years ago by
mhansen: we'd love to see your work-in-progress if it's available somewhere. I just spent some time reading the IPython docs again and thinking about how we could use it to implement the notebook hub and worker processes.
comment:13 Changed 10 years ago by
I've posted patches which let Sage work with IPython 0.12 as well as cleaning up some old messy code. These should be in a state where they're ready to review.
I haven't done anything with preparing an spkg yet.
comment:14 Changed 10 years ago by
- Description modified (diff)
I've updated the instructions in the description. Please correct if I messed something up.
comment:15 Changed 10 years ago by
- Status changed from new to needs_review
comment:16 Changed 10 years ago by
- Status changed from needs_review to needs_work
- Work issues set to spkg
comment:17 follow-up: ↓ 19 Changed 10 years ago by
Just curious: do your patches work with the current 0.12, or did you base them on the current git master (or does it not matter?). On the IPython mailing list, they mentioned that 0.12 had some big bugs, so they were thinking about cutting a quick 0.12.1. Also, they expect 0.13 in about a month.
comment:18 Changed 10 years ago by
I was thinking more about security today with the zmq messages. I think it may be more secure to have a default ipcontroller transport of IPC in a directory inside of $DOT_SAGE, rather than tcp over the loopback interface. That would be more secure in that random people on your computer wouldn't be able to try to register with the IPython hub. I emailed the IPython mailing list about it: http://thread.gmane.org/gmane.comp.python.ipython.devel/7789
What do you think?
comment:19 in reply to: ↑ 17 Changed 10 years ago by
Replying to jason:
Just curious: do your patches work with the current 0.12, or did you base them on the current git master (or does it not matter?). On the IPython mailing list, they mentioned that 0.12 had some big bugs, so they were thinking about cutting a quick 0.12.1. Also, they expect 0.13 in about a month.
I've looked at the current git master as I was making these changes, and I think these changes should work fine with what's on there. I'd be surprised if it made a difference.
comment:20 Changed 10 years ago by
I just tested things against git master and everything seems fine.
comment:21 Changed 10 years ago by
- Description modified (diff)
I'm trying to apply these to 5.0beta12. I think I probably got the order wrong in the description, so it would be great if you could check that. But regardless, none of the patches seem to apply to 5.0beta12 cleanly (each has rejects).
comment:22 Changed 10 years ago by
- Description modified (diff)
- Work issues changed from spkg to patches don't apply
I just tried applying to beta6 and the -scripts patch still had problems.
Changed 10 years ago by
comment:23 Changed 10 years ago by
The -scripts patch should apply fine to the scripts repository. I've rebased the other ones to work on the newer 5.0 beta's.
comment:24 Changed 10 years ago by
- Cc AlexanderDreyer added
After applying the patches to sage-5.0 beta12 I still have ip = IPython.ipapi.get()
at the end of sage/misc/edit_module.py
, which crashes Sage on startup.
comment:25 Changed 10 years ago by
I get a crash when I apply these (with a latest git IPython). I've posted the crash report at http://sage.math.washington.edu/home/jason/Sage_crash_report.txt
comment:26 Changed 10 years ago by
Is the zmq communication in ipython going through tcp/localhost sockets, or through UDS sockets? I really think we should use UDS sockets with the files in $DOTSAGE so that it is more secure (i.e., random other people on the computer can't connect to the kernel).
comment:27 Changed 10 years ago by
By default we should not use zeromq. For example, if you want to fork you should do so before creating a zmq context. The zmq context creates threads and open file descriptors.
Apart from that, you should either use IPC or TCP+magic cookie, much like the X window system. Implementation wise its probably easiest to allow any socket and alway check the magic cookie.
comment:28 Changed 10 years ago by
I don't think we have a choice of whether or not to use zmq with the new ipython. ZMQ is an integral part of how it communicates results from the backend to the frontend. They do have a system for signing messages and ignoring unsigned messages (is that the magic cookie thing?). UDS sockets is IPC, so we're good there. My problem with TCP+magic cookie is that it still allows someone else to attempt to connect to the various sockets, whereas using IPC, with the socket files in $DOTSAGE, filesystem permissions prevent others from even attempting to connect.
comment:29 Changed 10 years ago by
It may be that I'm misunderstanding how ipython is using zmq when you just execute 'ipython'. It may be that it uses in-process communication (using zmq) instead of opening ports on localhost. I've posted to the ipython-devel mailing list about this.
comment:30 Changed 10 years ago by
Given that you can build IPython without zeromq support it surely must be possible to run without?
I don't see the problem with tcp + magic cookie. Yes you can connect but your connection attempt will be ignored. And the difference in pyzmq to ipc + magic cookie is a single string. So why bother with ipc without magic cookie?
comment:31 Changed 10 years ago by
Thomas on the ipython mailing list confirmed that the plain ipython command doesn't use zmq, so that was a big misunderstanding I had about the new ipython. So never mind what I said about sockets.
Changed 10 years ago by
comment:32 Changed 10 years ago by
Okay, I fixed the problem with edit_module.
Sorry I wasn't clear about the default IPython not using zmq. There is a experimental console shell that uses two processes which can be accessed with "ipython console", but we aren't using that at all.
comment:33 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
- Work issues patches don't apply deleted
comment:34 Changed 10 years ago by
For the record, the patches now apply to my 5.0beta12 and Sage starts up.
comment:35 Changed 10 years ago by
When doing %edit, I edit the sage library file, but the file isn't copied over to the library after editing, so the changes don't take effect. This doesn't hold back this issue, though, since it didn't work before either.
comment:36 Changed 10 years ago by
Although zeromq is not, strictly speaking, necessary for IPython it might be of interest that I made zeromq and pyzmq spkgs at #12843
comment:37 Changed 10 years ago by
- Description modified (diff)
comment:38 Changed 10 years ago by
Not working on sage-5.0.rc0 (system Darwin 10.8.0). I installed the spkg and applied the patches in the given order and places, but still have the following error lauching Sage :
Traceback (most recent call last): File "/Users/pece/sage-5.0.rc0/local/bin/sage-ipython", line 19, in <module> from IPython.CrashHandler import CrashHandler ImportError: No module named CrashHandler
I even installed the patches from #12843 without any success.
comment:39 follow-up: ↓ 41 Changed 10 years ago by
I have OSX 10.6.8: Darwin Kernel Version 10.8.0
. I'm running the IPython git master and the patches, and it seems to work well. Did you do sage -b
to compile the patches? Maybe the problem is in the IPython 0.12.1->0.13 changes.
comment:40 Changed 10 years ago by
Hmmm, it looks like the scripts/ patch wasn't applied (the problematic line is deleted in the patch). Can you check again that trac_12719-scripts.patch is applied to the SAGE_ROOT/local/bin repository?
comment:41 in reply to: ↑ 39 Changed 10 years ago by
Replying to jason:
Maybe the problem is in the IPython 0.12.1->0.13 changes.
On sage-5.0.rc0, I managed to apply the three patches and sage starts correctly. So I guess it's not a matter of IPython 0.13 changes. Although, the sage prompt is not in color anymore (my file .sage/ipython/ipythonrc is set to dark background color).
Is the ipython notebook suppose to work? How should I test it?
Also, doing sage -ipython notebook
creates this error :
$ sage -ipython notebook Traceback (most recent call last): ... ImportError: The IPython Notebook requires tornado >= 2.1.0
comment:42 Changed 10 years ago by
The first patch (trac_12719.patch
) applies only with fuzz 2 on sage-5.0
comment:43 Changed 10 years ago by
- Keywords sd40.5 added
- Status changed from needs_review to needs_work
Lots of doctests fail because the display hook is not correctly set up. I'll have a look into this if nobody beats me to it...
comment:44 Changed 10 years ago by
- Description modified (diff)
The new ipython configuration files are once more incompatible with the previous ones. My patch versions the directory now (that is, it is named .sage/ipython-VERSION) and deletes the SAGE_ROOT/ipythonrc stuff. I think all sage-specific configuration should go into sage/misc/interface.py
comment:45 Changed 10 years ago by
Here is some reasonable documentation about Prefilter vs. InputSplitter: http://mail.scipy.org/pipermail/ipython-dev/2011-March/007334.html
comment:46 Changed 10 years ago by
- Description modified (diff)
- Reviewers set to Volker Braun
- Status changed from needs_work to needs_review
The trac_12719_ipython_fixes.patch switches form Prefilter to InputSplitter.
I'm giving a positive review to everything that Mike wrote.
comment:47 Changed 10 years ago by
With the new patch sage/misc/interpreter.py
has 100% coverage.
comment:49 Changed 10 years ago by
In Sage before this patch, the sage notebook, the Python command line, etc., we have:
for i in range(10): i
outputs the numbers up to 10.
With the new Ipython spkg, etc., we don't.
sage: for i in range(10): i sage: Exiting Sage (CPU time 0m0.07s, Wall time 2m3.22s). blastoff:sage wstein$ sage -ipython Python 2.7.2 (default, May 17 2012, 13:12:30) In [1]: for i in range(10): i In [2]:
I'm disappointed, especially because I thought this through and in sagews I recently wrote code so this works correctly in a single cell, even if there are several instances of this. It's a matter of passing the 'single' option to exec, as is documented in the Python reference. I wonder why they screwed this up in Ipython 0.12...?
Anyway, I'm not saying this is a show stopper at all - I want the new ipython in sage. I'm just a little concerned that literally the first thing I tried shows that they made what I consider a bad design choice. Hopefully it was just bad luck.
comment:50 Changed 10 years ago by
Cool -- I tried out a bunch of other things including the new notebook (sage -ipython notebook) and didn't run into any serious issues. So I'm happy with this from from a purely functional perspective. Also, deleting tons of ugly code (done in the patches), looks great.
comment:51 Changed 10 years ago by
It should be easy to get to get the correct functionality for for i in range(10): i
. The shell.run_ast_nodes
method just has to be called with interactivity="all"
instead of interactivity="last_expr"
.
comment:52 Changed 10 years ago by
Here's a review of Volker's ipython_fixes patch:
- (minor) Remove \ on line 1071
- Do one of the following:
- Fix/deprecate/remove sage-gdb-ipython since it depends on SAGE_CLEAN
- Just throw a deprecation warning with SAGE_CLEAN
- Turn autoindent back on.
- Why add debug=True and confirm_exit=False to default config.
- Load the startup file: line 1151
comment:53 Changed 10 years ago by
- Description modified (diff)
- Reviewers changed from Volker Braun to Volker Braun, Mike Hansen
1, 3, 5: done
4: I removed the debug, but I (and most others, I asked) definitely want to keep confirm_exit=False
. Yes I really want to quit, geez!
2: I'm in favor of a) and removed it.
I've also renamed trac_13013_ROOT_configuration_files.patch (wrong ticket number) and added a patch to auto-dedent and eval in loops. Now the following works:
sage: for i in range(3): i 0 1 2
comment:54 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_review to positive_review
Volker's changes look good to me.
comment:55 Changed 10 years ago by
- Description modified (diff)
Changed 10 years ago by
comment:56 Changed 10 years ago by
Mike's addition of trac_12719-crash.patch looks *great* to me as an addition.
comment:57 Changed 10 years ago by
- Status changed from positive_review to needs_work
Unfortunately, this doesn't apply to sage-5.1.beta1:
applying trac_12719.patch patching file doc/en/reference/misc.rst Hunk #1 succeeded at 30 with fuzz 2 (offset 2 lines). patching file sage/misc/interpreter.py Hunk #1 FAILED at 0 Hunk #3 FAILED at 271 Hunk #4 FAILED at 385 Hunk #5 FAILED at 449 4 out of 5 hunks FAILED -- saving rejects to file sage/misc/interpreter.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_12719.patch
comment:58 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
I've rebased this on top of #11817.
comment:59 Changed 10 years ago by
- Description modified (diff)
comment:60 Changed 10 years ago by
- Status changed from needs_review to needs_work
Now it applies but I get documentation errors:
dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py:docstring of sage.misc.interpreter:12: WARNING: Block quote ends without a blank line; unexpected unindent. dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py:docstring of sage.misc.interpreter:8: WARNING: Block quote ends without a blank line; unexpected unindent. dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py:docstring of sage.misc.interpreter:8: WARNING: Block quote ends without a blank line; unexpected unindent. dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py:docstring of sage.misc.interpreter:11: WARNING: Definition list ends without a blank line; unexpected unindent. dochtml.log:/padic/scratch/jdemeyer/merger/sage-5.1.beta3/local/lib/python2.7/site-packages/sage/misc/interpreter.py:docstring of sage.misc.interpreter:13: ERROR: Unexpected indentation.
comment:61 Changed 10 years ago by
Also, trac_12719-scripts.patch needs a commit message.
comment:62 Changed 10 years ago by
It seems that double question mark, like trace??
, doesn't bring up the source code anymore, but just brings up the docstring. It seems that trac_12719_ipython_fixes.patch broke this (double question marks work with just the first two patches, but not after that third patch).
comment:63 Changed 10 years ago by
- Description modified (diff)
comment:64 Changed 10 years ago by
- Description modified (diff)
Add two more patches to get the new IPython 0.13beta1 spkg above to work.
comment:65 Changed 10 years ago by
- Description modified (diff)
comment:66 Changed 10 years ago by
- Description modified (diff)
comment:67 Changed 10 years ago by
- Work issues set to ?? doesn't work
comment:68 Changed 10 years ago by
- Work issues changed from ?? doesn't work to ?? doesn't show code
comment:69 Changed 10 years ago by
- Description modified (diff)
comment:70 Changed 10 years ago by
- Description modified (diff)
comment:71 Changed 10 years ago by
I rebased trac_12719_ROOT_configuration_files.patch to sage 5.1beta6
comment:72 Changed 10 years ago by
- Description modified (diff)
- Reviewers changed from Volker Braun, Mike Hansen to Volker Braun, Mike Hansen, Jason Grout
- Summary changed from Upgrade to IPython 0.12 to Upgrade to IPython 0.13
comment:73 Changed 10 years ago by
I upgraded the spkg to 0.13, which was just released today.
comment:74 Changed 10 years ago by
- Description modified (diff)
See #12167 for a related ticket: how we deal with IPython configuration files.