Opened 2 years ago

Closed 19 months ago

Last modified 18 months ago

#12719 closed enhancement (fixed)

Upgrade to IPython 0.13

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: sage-5.7.beta1
Authors: Mike Hansen, Volker Braun, Jason Grout, Jeroen Demeyer Reviewers: Volker Braun, Mike Hansen, Jason Grout, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13459, #9191, #13717, #13963 Stopgaps:

Description (last modified by jdemeyer)

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.1 spkg at http://boxen.math.washington.edu/home/jdemeyer/spkg/ipython-0.13.1.spkg

apply:

To the scripts repository:

To the root repository:

To the sage library:

So:

cd SAGE_ROOT

./sage --hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_ROOT_configuration_files.patch
./sage --hg qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-root-spkg-install.patch

./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-scripts-vb.patch
./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_remove_sage_gdb_ipython.patch
./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-test-displayhook.patch
./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719_run_cython.patch
./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-hgignore.patch
./sage --hg -R local/bin qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_purge_ipy_profile_sage.patch

./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-rebased-to-13211-vb.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-move_to_preparser_rebased.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_ipython_fixes.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719_dedent.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-crash.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-cellmagics013.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-newipython-take2.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-displayhook.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-fix-transformer.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-5.4rc1.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-remove-plugin.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-displayhook-library.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-pager.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-preparse-doctest.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac_12719-gcd-repr.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719-system.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/trac-12719-always-preparse.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719_ipython_test.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12719/12719_ipython_no_history.patch

./sage -i http://sage.math.washington.edu/home/jason/ipython-0.13.1.spkg

./sage -br

Attachments (35)

trac_12719-move_to_preparser.patch (13.2 KB) - added by mhansen 2 years ago.
trac_12719.patch (63.6 KB) - added by mhansen 2 years ago.
trac_13013_ROOT_configuration_files.patch (40.8 KB) - added by vbraun 2 years ago.
Updated patch
trac_12719_dedent.patch (2.9 KB) - added by vbraun 2 years ago.
Initial patch
trac_12719-crash.patch (1.6 KB) - added by mhansen 2 years ago.
trac_12719_ipython_fixes.patch (37.1 KB) - added by jason 2 years ago.
Updated patch
12719-newipython-take2.patch (60.3 KB) - added by jason 2 years ago.
12719-displayhook.patch (8.2 KB) - added by jason 2 years ago.
12719-fix-transformer.patch (1.6 KB) - added by jason 2 years ago.
trac_12719-5.1beta1.patch (63.8 KB) - added by jason 23 months ago.
rebased to 5.4rc1
trac_12719_remove_sage_gdb_ipython.patch (567 bytes) - added by jason 23 months ago.
rebased to 5.4rc1
12719-remove-plugin.patch (1.9 KB) - added by jason 23 months ago.
12719-preparse-magic.patch (728 bytes) - added by jason 23 months ago.
12719-test-displayhook.patch (705 bytes) - added by jason 23 months ago.
apply to scripts repository in local/bin/
12719-displayhook-library.patch (1.9 KB) - added by jason 23 months ago.
trac_12719-pager.patch (1.7 KB) - added by jhpalmieri 23 months ago.
trac_12719-move_to_preparser_rebased.patch (13.3 KB) - added by jhpalmieri 23 months ago.
trac_12719-preparse-doctest.patch (1.4 KB) - added by jhpalmieri 23 months ago.
trac_12719-gcd-repr.patch (719 bytes) - added by jhpalmieri 22 months ago.
trac_12719-rebased-to-13211.patch (64.3 KB) - added by jhpalmieri 22 months ago.
12719_run_cython.patch (581 bytes) - added by jdemeyer 22 months ago.
trac_12719-hgignore.patch (404 bytes) - added by jhpalmieri 22 months ago.
trac_12719-cellmagics013.patch (687 bytes) - added by jason 22 months ago.
Apply to sage repository
trac_12719-scripts.patch (3.9 KB) - added by jason 22 months ago.
rebased to 5.4rc1
12719-5.4rc1.patch (781 bytes) - added by jason 22 months ago.
12719-system.patch (2.9 KB) - added by jason 22 months ago.
apply to sage repository
trac_12719-root-spkg-install.patch (744 bytes) - added by jhpalmieri 22 months ago.
root repo
trac-12719-always-preparse.patch (1.6 KB) - added by jason 21 months ago.
apply to sage library
trac_12719-scripts-vb.patch (3.9 KB) - added by vbraun 20 months ago.
Updated patch
trac_12719-rebased-to-13211-vb.patch (63.8 KB) - added by jdemeyer 20 months ago.
Rediffed patch
trac_12719-ipythondir013.patch (1000 bytes) - added by jason 20 months ago.
rebased to 5.4rc1
trac_12719_ROOT_configuration_files.patch (41.8 KB) - added by jdemeyer 20 months ago.
Rebased to sage-5.7.beta0
trac_12719_purge_ipy_profile_sage.patch (1.0 KB) - added by vbraun 20 months ago.
Initial patch
12719_ipython_test.patch (1.2 KB) - added by jdemeyer 19 months ago.
12719_ipython_no_history.patch (1.2 KB) - added by jdemeyer 19 months ago.

Download all attachments as: .zip

Change History (318)

comment:1 Changed 2 years ago by jhpalmieri

See #12167 for a related ticket: how we deal with IPython configuration files.

comment:2 follow-up: Changed 2 years ago by fbissey

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 2 years ago by kini

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 2 years ago by fbissey

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 2 years ago by iandrus

  • Cc iandrus added

comment:6 Changed 2 years ago by mhansen

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 2 years ago by mhansen

This will fix #4413.

comment:8 Changed 2 years ago by kini

Great, I'm glad you're on the case! :)

comment:9 Changed 2 years ago by vbraun

  • Cc vbraun added

comment:10 Changed 2 years ago by vbraun

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 2 years ago by kini

  • Cc jason added

CCing Jason as he expressed interest on sage-devel.

comment:12 Changed 2 years ago by jason

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 2 years ago by mhansen

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 2 years ago by jason

  • Authors set to Mike Hansen
  • Description modified (diff)

I've updated the instructions in the description. Please correct if I messed something up.

comment:15 Changed 2 years ago by jason

  • Status changed from new to needs_review

comment:16 Changed 2 years ago by jason

  • Status changed from needs_review to needs_work
  • Work issues set to spkg

comment:17 follow-up: Changed 2 years ago by 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.

comment:18 Changed 2 years ago by jason

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 2 years ago by mhansen

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 2 years ago by mhansen

I just tested things against git master and everything seems fine.

comment:21 Changed 2 years ago by jason

  • 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 2 years ago by jason

  • 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 2 years ago by mhansen

comment:23 Changed 2 years ago by mhansen

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 2 years ago by AlexanderDreyer

  • 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 2 years ago by jason

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 2 years ago by jason

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 2 years ago by vbraun

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 2 years ago by jason

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 2 years ago by jason

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 2 years ago by vbraun

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 2 years ago by jason

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 2 years ago by mhansen

comment:32 Changed 2 years ago by mhansen

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 2 years ago by jason

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues patches don't apply deleted

comment:34 Changed 2 years ago by jason

For the record, the patches now apply to my 5.0beta12 and Sage starts up.

comment:35 Changed 2 years ago by jason

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 2 years ago by vbraun

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 2 years ago by jason

  • Description modified (diff)

comment:38 Changed 2 years ago by PierreCagne

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: Changed 2 years ago by jason

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.

Last edited 2 years ago by jason (previous) (diff)

comment:40 Changed 2 years ago by jason

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 ; follow-up: Changed 2 years ago by slabbe

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 2 years ago by vbraun

The first patch (trac_12719.patch) applies only with fuzz 2 on sage-5.0

comment:43 Changed 2 years ago by vbraun

  • 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 2 years ago by vbraun

  • 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 2 years ago by vbraun

Here is some reasonable documentation about Prefilter vs. InputSplitter: http://mail.scipy.org/pipermail/ipython-dev/2011-March/007334.html

Changed 2 years ago by vbraun

Updated patch

comment:46 Changed 2 years ago by vbraun

  • Authors changed from Mike Hansen to Mike Hansen, Volker Braun
  • 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 2 years ago by vbraun

With the new patch sage/misc/interpreter.py has 100% coverage.

comment:48 Changed 2 years ago by was

never mind.

Last edited 2 years ago by was (previous) (diff)

comment:49 follow-up: Changed 2 years ago by was

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 2 years ago by was

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 2 years ago by mhansen

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 2 years ago by mhansen

Here's a review of Volker's ipython_fixes patch:

  1. (minor) Remove \ on line 1071
  2. Do one of the following:
    1. Fix/deprecate/remove sage-gdb-ipython since it depends on SAGE_CLEAN
    2. Just throw a deprecation warning with SAGE_CLEAN
  3. Turn autoindent back on.
  4. Why add debug=True and confirm_exit=False to default config.
  5. Load the startup file: line 1151

Changed 2 years ago by vbraun

Initial patch

comment:53 Changed 2 years ago by vbraun

  • 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 2 years ago by mhansen

  • Description modified (diff)
  • Status changed from needs_review to positive_review

Volker's changes look good to me.

comment:55 Changed 2 years ago by mhansen

  • Description modified (diff)

Changed 2 years ago by mhansen

comment:56 Changed 2 years ago by was

Mike's addition of trac_12719-crash.patch looks *great* to me as an addition.

comment:57 Changed 2 years ago by jdemeyer

  • 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 2 years ago by mhansen

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I've rebased this on top of #11817.

comment:59 Changed 2 years ago by jhpalmieri

  • Description modified (diff)

comment:60 Changed 2 years ago by jdemeyer

  • 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 2 years ago by jdemeyer

Also, trac_12719-scripts.patch needs a commit message.

comment:62 Changed 2 years ago by jason

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).

Last edited 2 years ago by jason (previous) (diff)

comment:63 Changed 2 years ago by jason

  • Description modified (diff)

comment:64 Changed 2 years ago by jason

  • Description modified (diff)

Add two more patches to get the new IPython 0.13beta1 spkg above to work.

comment:65 Changed 2 years ago by jason

  • Description modified (diff)

comment:66 Changed 2 years ago by jason

  • Description modified (diff)

comment:67 Changed 2 years ago by jason

  • Work issues set to ?? doesn't work

comment:68 Changed 2 years ago by jason

  • Work issues changed from ?? doesn't work to ?? doesn't show code

comment:69 Changed 2 years ago by jason

  • Description modified (diff)

comment:70 Changed 2 years ago by jason

  • Description modified (diff)

comment:71 Changed 2 years ago by jason

comment:72 Changed 2 years ago by jason

  • Authors changed from Mike Hansen, Volker Braun to Mike Hansen, Volker Braun, Jason Grout
  • 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 2 years ago by jason

I upgraded the spkg to 0.13, which was just released today.

comment:74 Changed 2 years ago by jason

  • Description modified (diff)

comment:75 in reply to: ↑ 49 Changed 2 years ago by jason

Replying to was:

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.

Fernando Perez responded:

BTW, regarding William's comment, that was not a mistake or omission: it was a very deliberate choice we made after lots of thought. Given that we manage numbered prompts and history, and that matplotlib produces so many plots as side effects, after much experimentation we found it worked much better to only execute with 'interactive' mode the last block.

Sage doesn't distinguish stdout from python sys.displayhook, so the opposite behavior may be more desirable, but we actually consider that a 'bad design choice' of sage, because it loses an important distinction and the cache management of output history.

So in summary: our choice is actually sensible and consistent given IPython's internal design :)

comment:76 follow-up: Changed 2 years ago by jason

Fernando: I looked in the IPython source, and it seems that IPython is a little more subtle than mentioned above. By default, it only executes the last block as interactive (compile in 'single' mode) if it is an expression. In Sage, we (by default) execute the last block always in 'single' mode. I think that is the difference here.

Given that (and if I understood correctly), could you explain again your reasoning for choosing to not compile in 'single' mode if it isn't an expression?

Note to self for future reference: for i in range(10): i invokes sys.displayhook 10 times to display i if compiled in 'single' mode.

comment:77 Changed 2 years ago by jason

Also, note to William: it is an easy option to set to have IPython *always* execute the last block in interactive mode. It's just not the default.

comment:78 in reply to: ↑ 76 Changed 2 years ago by jason

Replying to jason:

Fernando: I looked in the IPython source, and it seems that IPython is a little more subtle than mentioned above. By default, it only executes the last block as interactive (compile in 'single' mode) if it is an expression. In Sage, we (by default) execute the last block always in 'single' mode. I think that is the difference here.

Given that (and if I understood correctly), could you explain again your reasoning for choosing to not compile in 'single' mode if it isn't an expression?

And here is Fernando's response:

Precisely so that if the last block has a loop, it doesn't produce multiple Out [NN] cells. We want to keep a one-to-one mapping between a cell and its output. For example it's quite common to write code like:

for i in x:
  plot(x[i], label='blah_i')

If we compile that as 'single', we'd get all the Matplotlib line collection results piling up at the bottom, one for each plot call.

We played a *lot* with various options, and settled on this behavior after much experimentation. After 2 years of using it, I remain convinced it's the right solution *for us*. Not to say that Sage can't make a different choice that works better for you guys, of course!

comment:79 Changed 2 years ago by jason

To change this, we just need to set ast_node_interactivity to "last" in the InteractiveShell.

Last edited 2 years ago by jason (previous) (diff)

comment:80 in reply to: ↑ 2 Changed 2 years ago by schilly

Replying to fbissey:

Would be nice but we need some porting, here is what typically happen if you try to use ipython-0.12 with 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

Hi fbissey, sorry for hijacking this a bit, but i had exactly the same problem over and over again. What I just found out (and I think it really helps) is to temporarily disable the paths containing ".local" in sys.path in the sage-ipython file. The reason is, that ipython seems to import the "site package" (disable it for pure python via "-S"), because that's where my ipython 0.13 is.

So, by adding this to sage-ipython you I have both at the same time:

~~~ after import sys
oldpath = sys.path
sys.path = filter(lambda s : '.local' not in s, sys.path)
import IPython
sys.path = oldpath

comment:81 Changed 2 years ago by jason

  • Description modified (diff)

Changed 2 years ago by jason

Updated patch

comment:82 Changed 2 years ago by jason

I rebased the only rejected patch for 5.2beta1

comment:83 Changed 2 years ago by was

My 2 cents, after I forced myself to use this for the last few weeks on my laptop. I have found the new ipython to be pretty annoying. The cut and paste behavior and lack of autoindent at the command line is pretty painful. The thing that made me finally give up and revert to the old 0.10 ipython was when I hit control-c at the wrong moment when starting Sage, and somehow the ipython history sqlite database got in a weird locked state. Nothing I could think to do (including deleting $HOME/.sage/ipython) got this unstuck. Maybe rebooting would have worked; I don't know. I'm concerned about how stable and well-tested in the wild the new ipython is. Proceed with caution.

comment:84 Changed 2 years ago by vbraun

Working cut&past and working autoindent are contradictory goals. The terminal just sees a stream of characters, you either inject indentation or not. AFAIR I turned off auto-indent, we could turn that back on. Breaks pasting of indented text, of course.

The ipython dir is now versioned, you should have deleted $DOT_SAGE/ipython-0.12 / $DOT_SAGE/ipython-0.13

comment:85 Changed 2 years ago by vbraun

PS: Maybe you can upload your broken ipython dir somewhere, this would be a good testcase to catch the sqlite error...

comment:86 follow-up: Changed 2 years ago by kini

IMO, leave auto-indent on, and use %paste or %cpaste when you want to paste. %paste tries to automatically get the contents of your clipboard. If that's not working for whatever reason, %cpaste creates a prompt where you can paste text verbatim, which IPython doesn't read until you end input with Ctrl+D or a line containing only the text "--".

comment:87 in reply to: ↑ 86 ; follow-up: Changed 2 years ago by vbraun

Replying to kini:

IMO, leave auto-indent on, and use %paste or %cpaste when you want to paste.

For the record, thats not how I use the commandline. I paste code all the time, but I almost never enter multi-line statements manually. So I'd rather have autoindent off by default (and no need for %cpaste), and use %autoindent if I want it on. Though thats of course a matter of personal preferences, and I'm happy with whatever the majority prefers...

comment:88 in reply to: ↑ 87 Changed 2 years ago by was

Replying to vbraun:

Replying to kini:

IMO, leave auto-indent on, and use %paste or %cpaste when you want to paste.

For the record, thats not how I use the commandline. I paste code all the time, but I almost never enter multi-line statements manually. So I'd rather have autoindent off by default (and no need for %cpaste), and use %autoindent if I want it on. Though thats of course a matter of personal preferences, and I'm happy with whatever the majority prefers...

At least it is configurable, right? I think it is best not to change the default from what it is already in all previous versions of Sage, whether or not that is what the majority prefers (unless it is a huge majority). I'm not fan of changing UI's on people unless it is absolutely necessary.

comment:89 Changed 2 years ago by vbraun

Well IPythons defaults changed, we didn't just change stuff for the fun of it. For starters, vanilla IPython now disallows any superfluous indentation, i.e.

In [1]: 1
Out[1]: 1

In [2]:  1
IndentationError: unexpected indent (<ipython-input-2-0066badbf200>, line 1)

comment:90 Changed 2 years ago by jhpalmieri

Two more issues:

  • when Sage starts, any lines in my init.sage file are echoed to the screen after the first "sage:" prompt. They aren't executed until I hit the return key:
    ----------------------------------------------------------------------
    | Sage Version 5.2, Release Date: 2012-07-25                         |
    | Type "notebook()" for the browser-based notebook interface.        |
    | Type "help()" for help.                                            |
    ----------------------------------------------------------------------
    sage: # Here is the beginning of init.sage.
    
    2+2
    
    # Here is another comment
    
    a = 5
    
    # Here is the end of init.sage.
    
    At this point, hitting <RETURN> inserts a 4, a newline, and the "sage:" prompt, and a is defined to be 5.
  • when I evaluate or plot a graphics object, two plot windows are opened:
    sage: P = plot(sin(x), (x, 0, 10))
    sage: P  # opens up two windows showing the same plot. Note also the blank line.
    
    sage: plot(P) # also opens up two windows.
    
    sage: show(P)  # opens up only one window.
    

comment:91 Changed 2 years ago by iandrus

Regarding indentation, autoindent being on breaks sage inside Emacs (dumb terminal). So I think it should be off, at least in a dumb terminal, until/unless I can figure out how to fix this in Emacs. I haven't really looked into it at all, so it may be easy to fix inside Emacs by setting some environment variables or something. See #10504.

comment:92 Changed 2 years ago by jason

+1 for autoindent being on by default. That helps what is probably the vast majority of the cases---interactively typing into the command line.

comment:93 Changed 2 years ago by broken_symlink

I'm having a weird problem when I use IPython.parallel in sage. Here's what happens:

from IPython.parallel import Client
rc = Client('/home/seshu/.sage/ipython-new/profile_default/security/ipcontroller-client.json')
dview = rc[:]
dview.map_sync(lambda x: x**10, range(32))
[0:apply]: 
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)<string> in <module>()
<ipython-input-4-ad199b8d76dc> in <lambda>(x)
NameError: global name 'Integer' is not defined

If I do anything that doesn't involve integers like dview.map_sync(lambda x: "hi", range(32)), it works fine. Sorry if this isn't the appropriate place to ask these questions. I wasn't sure where to ask since this is a bit bleeding edge. I would like to be able to use IPython.parallel from the sage notebook if possible.

comment:94 Changed 2 years ago by broken_symlink

The ipython engines need to import the sage modules. The fix is dview.execute("from sage.all import *"). Also I did sage -sh ipcluster start.

comment:95 Changed 2 years ago by jason

Just FYI, for the *next* version (so not an issue on this ticket), https://github.com/ipython/ipython/pull/2299 broke Sage, I think by getting rid of PrefilterTransformer? tricks.

comment:96 Changed 2 years ago by jason

So I think that the only issue with this ticket is turning back on auto-indentation and getting ?? to show the source code? Any other issues?

I think the parallel issues above should be moved to another ticket.

Last edited 2 years ago by jason (previous) (diff)

comment:97 Changed 2 years ago by jason

Note: ?? shows the source code fine, it seems, in the notebook (at least there doesn't seem to be a regression; ?? shows source when doing tab completion, but not on evaluating, just like before the python upgrade). It is a regression on the command line, though.

comment:98 Changed 2 years ago by jhpalmieri

There are the issues I mentioned above. Can anyone else reproduce them?

comment:99 Changed 2 years ago by broken_symlink

The parallel issues only seem to effect the notebook. I've successfully gotten ipython.parallel and sage to run on a small 24 core amazon ec2 cluster, using ssh with the ipython 0.13.1 branch on github. According to the ipython guys, the ssh launcher in 0.13 is broken. I kinda have some idea of whats going with the parallel issues and the notebook as well, but i'm not sure how to fix it. I can write something for how to setup sage with ipython engines started over ssh if this package gets updated to 0.13.1.

comment:100 Changed 2 years ago by jason

Well, hopefully this gets merged before 0.13.1 comes out.

comment:101 Changed 2 years ago by jason

jhpalmieri, I can confirm both of the issues that you saw. I'm seeing them too.

comment:102 Changed 2 years ago by jason

  • Work issues changed from ?? doesn't show code to ?? doesn't show code, init.sage displayed, double plot windows, autoindent disabled

comment:103 Changed 2 years ago by jason

  • Work issues changed from ?? doesn't show code, init.sage displayed, double plot windows, autoindent disabled to ?? doesn't show code, init.sage displayed, double plot windows

actually, it looks like autoindent was turned back on: http://trac.sagemath.org/sage_trac/ticket/12719#comment:52

comment:104 Changed 2 years ago by jason

Does anyone know where in the code init.sage is run? I've searched the entire codebase for "init.sage" and I'm not finding anything that looks like it is responsible for running the file.

comment:105 Changed 2 years ago by jason

Ah, I found it. It's the last line in sage/misc/interpreter.py, the one that says self.shell.run_cell('%%load "%s"'%startup_file)

comment:106 Changed 2 years ago by jason

Okay, it looks like the problem is that we punt to the ipython load, which *pastes* the content so you can edit it first: http://ipython.org/ipython-doc/dev/interactive/qtconsole.html#load

comment:107 Changed 2 years ago by jason

So our conclusion on https://github.com/ipython/ipython/pull/2299 was that Sage should make %load an alias for IPython's %run, and beef up IPython's %run to be able to handle remote files. In fact, make IPython's %run use the same simple mechanism as IPython's %load would be great (though may not be possible---%run uses file-specific features in python and execfile, etc.)

Last edited 2 years ago by jason (previous) (diff)

comment:108 Changed 2 years ago by jason

Is there a reason we didn't use a profile config file for all of our customizations?

I've started a different approach to the customizations we use here: https://github.com/jasongrout/sage-ipython-profile

put these into the .sage/ipython-new directory and then start with sage -ipython --profile=sage

comment:109 Changed 2 years ago by jason

  • Description modified (diff)

Changed 2 years ago by jason

comment:110 Changed 2 years ago by jason

Okay, I think my "take2" patch significantly cleans things up and makes things more modular and more "IPythonic".

In particular, this seems to work nicely: do sage -ipython notebook to launch the ipython notebook, then evaluate %load_ext sage.misc.sage_extension. All preparsing is turned on, etc. It's great!

There should probably be a bit more documentation in the sage_extension file, and full doctests should be run. But I think it's ready for other people to hammer on it too.

Here is one issue: something broke in sage -startuptime. I'm not sure it's IPython's fault, though.

Oh, and I was working from IPython master, not IPython 0.13. I hope that doesn't put a wrench in the works...

Last edited 2 years ago by jason (previous) (diff)

comment:111 Changed 2 years ago by jason

Preliminary tests seem to indicate that these patches and the new take work just fine with IPython 0.13 (at least when I revert my IPython git repository). So it's still probably ready for more people to hammer on this.

comment:112 Changed 2 years ago by jason

  • Work issues changed from ?? doesn't show code, init.sage displayed, double plot windows to ?? doesn't show code

My take2 patch seems to have solved the double plot window problem. It also solved the init.sage/load problems, as well as the ?? problem. So that was all of the issues listed...

Last edited 2 years ago by jason (previous) (diff)

comment:113 Changed 2 years ago by jason

  • Work issues ?? doesn't show code deleted

Changed 2 years ago by jason

comment:114 Changed 2 years ago by jason

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Since we now hook into the IPython display hook, we can delete our special display hook.

comment:115 Changed 2 years ago by jason

  • Description modified (diff)

Fixed the startup issue with new patch to scripts repository

comment:116 Changed 2 years ago by jason

  • Description modified (diff)

Changed 2 years ago by jason

comment:117 Changed 2 years ago by jason

  • Description modified (diff)

Fix a transforming issue: see http://mail.scipy.org/pipermail/ipython-dev/2012-September/010329.html or the commit message.

comment:118 Changed 2 years ago by jdemeyer

  • Dependencies set to #13459
  • Status changed from needs_review to needs_work

This needs to be rebased to sage-5.4.rc0, in particular #13459.

comment:119 Changed 23 months ago by jason

Also, #13361 includes the change in the startuptime.py patch, so we can delete it.

comment:120 Changed 23 months ago by jason

  • Description modified (diff)

Changed 23 months ago by jason

rebased to 5.4rc1

Changed 23 months ago by jason

rebased to 5.4rc1

comment:121 Changed 23 months ago by jason

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Add one more patch so we don't change directories anymore.

comment:122 follow-up: Changed 23 months ago by jason

Something that seems to work fine now is starting up sage in the devel/sage directory. So it seems that the issue discussed in #13361 is fixed in this ipython upgrade.

Changed 23 months ago by jason

comment:123 Changed 23 months ago by jason

  • Description modified (diff)

comment:124 Changed 23 months ago by jason

Okay, I think this may be ready for review again.

It's a big enough change that maybe it would be good to shoot for one of the first patches in the 5.5 release, so it gets plenty of testing???

comment:125 in reply to: ↑ 41 Changed 23 months ago by jhpalmieri

Replying to slabbe:

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

I get this too. Should I just install tornado somehow, or should this be fixed as part of this ticket?

Edit: after installing tornado (with sage --sh and easy_install tornado), it said I had to install pyzmq. So I did that, and now the notebook runs. It looks very nice, and feels pretty snappy, maybe snappier than the Sage notebook.

Last edited 23 months ago by jhpalmieri (previous) (diff)

comment:126 Changed 23 months ago by jason

Yes, tornado and zmq (pyzmq) are required for the ipython notebook. That's not part of this ticket.

And yes, the ipython notebook is very nice. After loading it up, try doing

%load_ext sage.misc.sage_extension

to get some sage goodness, like the preparser, etc.!

Last edited 23 months ago by jason (previous) (diff)

comment:127 in reply to: ↑ 122 ; follow-up: Changed 23 months ago by jdemeyer

Replying to jason:

Something that seems to work fine now is starting up sage in the devel/sage directory.

FYI: this was fixed by #13459.

comment:128 in reply to: ↑ 127 Changed 23 months ago by jason

Replying to jdemeyer:

Replying to jason:

Something that seems to work fine now is starting up sage in the devel/sage directory.

FYI: this was fixed by #13459.

But I reverted the "import sage" that #13459 does in trac_12719-scripts.patch, and it still works fine for me (running sage in devel/sage and then import sage actually imports the site-packages sage). Maybe IPython changed its handling of sys.path. What I'm saying is that we should test that my reverting of part of #13459 (the part where we imported sage in sage-ipython) actually does what I think it does.

Last edited 23 months ago by jason (previous) (diff)

comment:129 Changed 23 months ago by jhpalmieri

I'm getting doctest failures because of how lists of matrices are printed. For example, with matrix2.pyx:

File ".../sage-5.4.rc1/devel/sage/sage/matrix/matrix2.pyx", line 11627:
    sage: (d, u, v)
Expected:
    (
    [     1      0]  [ 1  0]  [ 1 -w]
    [     0 -w + 9], [-w  1], [ 0  1]
    )
Got:
    ([     1      0]
    [     0 -w + 9], [ 1  0]
    [-w  1], [ 1 -w]
    [ 0  1])

That file has 7 failures altogether. Is this because you turned off Sage's displayhook?

comment:130 Changed 23 months ago by jason

Interesting. I thought I used Sage's displayhook (I know I was at one time anyway), but I've also noticed the same problems on my install with printing tuples of matrices and it's been frustrating to me. I'll take a look at it.

comment:131 Changed 23 months ago by jhpalmieri

12719-displayhook.patch looks relevant, in particular removing the lines

import sage.misc.displayhook  
sage.misc.displayhook.install() 

and removing the corresponding functions from sage/misc/displayhook.py. But I've never worked with this aspect of Sage before.

comment:132 Changed 23 months ago by jhpalmieri

When I use either time or %time, the rest of the line doesn't seem to be preparsed: try

sage: srange(1/10, 1/2, 1/10)
[1/10, 1/5, 3/10, 2/5]

vs.

sage: %time srange(1/10, 1/2, 1/10)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...
ValueError: srange() step argument must not be zero

Or even

sage: %time 1/10
CPU times: user 0.00 s, sys: 0.00 s, total: 0.00 s
Wall time: 0.00 s
0  <-- note the answer!

comment:133 Changed 23 months ago by jason

The displayhook is activated when Sage is run (try running those matrix doctests from the command line). The problem is that the doctesting framework does *not* use the interactive version of Sage; it seems to use straight python. So the display hook is not set.

It seems that the display hook used to be set when just importing Sage as a library, maybe? If so, I think that's the wrong approach. Importing a library shouldn't obliterate the display hook.

So it sounds like we need to make the doctesting framework load the right Sage things, like the preparser, the display hook, etc.

comment:134 Changed 23 months ago by jason

I'll take a look at %time. That indeed seems to be a problem where the preparser isn't being run.

comment:135 Changed 23 months ago by jason

  • Description modified (diff)

For some reason, the preparser explicitly turned itself off for magic lines. I'm not sure why but I'm posting a patch to fix that.

Changed 23 months ago by jason

comment:136 Changed 23 months ago by jason

  • Description modified (diff)

comment:137 Changed 23 months ago by jason

John, you know the doctest framework a lot better than I do. If you start up sage-ipython and load the sage extension, it will install the display hook, import Sage, etc. John, do you know how to get the doctest runner to execute this as part of ipython: get_ipython().extension_manager.load_extension('sage.misc.sage_extension')

grout@tiny:~/sage% sage -ipython -ic "get_ipython().extension_manager.load_extension('sage.misc.sage_extension')"                    E:128
Python 2.7.3 (default, Oct  9 2012, 11:53:24) 
Type "copyright", "credits" or "license" for more information.

IPython 0.13 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: (identity_matrix(3), identity_matrix(5))
Out[1]: 
(
         [1 0 0 0 0]
         [0 1 0 0 0]
[1 0 0]  [0 0 1 0 0]
[0 1 0]  [0 0 0 1 0]
[0 0 1], [0 0 0 0 1]
)

comment:138 Changed 23 months ago by jhpalmieri

I guess one reason to turn off the preparser for magic lines is so that in a line like

%ed file1.py

the "1" doesn't get preparsed. I don't know how the old "time" command was written; did it just call "%time"? If so, maybe that was to allow lines starting with "time" to be preparsed.

For doctesting, I don't know if IPython is used at all during doctesting. If it's possible to turn on this extension using pure Python, we can add it at the top of each file as it's doctested, with a patch like this to local/bin/sage-doctest:

  • sage-doctest

    diff --git a/sage-doctest b/sage-doctest
    a b def extract_doc(file_name, library_code= 
    490490 
    491491"""  % os.path.join(os.getcwd(), file_name) 
    492492    s += "from sage.all_cmdline import *; \n" 
     493    s += "ADD NEW CODE HERE\n" 
    493494    s += "import sage.plot.plot; sage.plot.plot.DOCTEST_MODE=True\n"  # turn off image popup 
    494495    s += r""" 
    495496def warning_function(f): 

Changed 23 months ago by jason

apply to scripts repository in local/bin/

Changed 23 months ago by jason

comment:139 Changed 23 months ago by jason

  • Description modified (diff)

Updated instructions for new display hook patch that sets the display hook when doctesting.

comment:140 Changed 23 months ago by jason

For the %time command, I filed an issue with IPython a while ago: https://github.com/ipython/ipython/pull/2403

For now, it appears like we'll have to ship that patch ourselves.

comment:141 Changed 23 months ago by jason

  • Work issues set to ship https://github.com/ipython/ipython/pull/2403

comment:142 Changed 23 months ago by jhpalmieri

The matrix-printing seems to work now, but I'm seeing a few other doctest failures:

sage -t  --long -force_lib devel/sage/sage/interfaces/sage0.py
**********************************************************************
File "/scratch/palmieri/12719/sage-5.4.rc1/devel/sage-main/sage/interfaces/sage0.py", line 489:
    sage: sage0(4).gcd
Expected:
    <built-in method gcd of sage.rings.integer.Integer object at 0x...>
Got:
    <function gcd>
**********************************************************************

and several failures in sage_extension.py, which would take up too much room to include here.

comment:143 Changed 23 months ago by jhpalmieri

I think the errors in sage_extension.py are because file names in %runfile foo are being preparsed: for example, the first error message is pretty long, but it ends with

IOError: did not find file '/home/palmieri/.sage/temp/sage.math.washington.edu/Integer(21508)/dir_0/run_cell.py' to load or attach

Note that Integer(21508) in the file name; this should presumably just be 21508. This preparsing doesn't happen if the number is mixed in with letters, but for files or directories with purely numeric names, it seems to pop up. So from the command line:

sage: %runfile hello2.py   # works
sage: %runfile 123/hello2.py   # fails
sage: %runfile /home/palmieri/123/hello2.py   # fails

The same thing happens with %attach, and I assume also with the other magic functions which take file names as arguments.

comment:144 Changed 23 months ago by jhpalmieri

I don't understand the sage0.py failure. Does the new IPython have a different default print representation for some objects? Before the patches here:

sage: a = 4
sage: a.gcd
<built-in method gcd of sage.rings.integer.Integer object at 0x480e330>
sage: a.gcd?
Type:		builtin_function_or_method
Base Class:	<type 'builtin_function_or_method'>
String Form:	<built-in method gcd of sage.rings.integer.Integer object at 0x480e330>
Namespace:	Interactive
Definition:	a.gcd(self, n)
Docstring:
     ...

With the patches:

sage: a = 4
sage: a.gcd
<function gcd>
sage: a.gcd?
Type:       builtin_function_or_method
String Form:<built-in method gcd of sage.rings.integer.Integer object at 0x5b7c5a0>
Definition: a.gcd(self, n)
Docstring:
   ...

With the old version, the print rep for a.gcd is the same as what's labeled the "String Form" when you do a.gcd?. With the new version, the print rep has changed but the "String Form" has not. I'm also confused by this behavior (with the patches):

sage: a = 4
sage: G = a.gcd
sage: G.__repr__()
'<built-in method gcd of sage.rings.integer.Integer object at 0x4b685a0>'
sage: G
<function gcd>
sage: str(G)
'<built-in method gcd of sage.rings.integer.Integer object at 0x4b685a0>'

Why isn't G printed using its __repr__ method?

I've tried out some other objects (e.g., matrices and elements of the Steenrod algebra) and my guess is that the print representations for methods defined in pyx files are getting changed to <function ...>, while those defined in py files haven't changed. Any ideas why? Looking at ipython-0.10.2.p1/src/IPython/UserConfig/ipythonrc, I see the configuration option object_info_string_level 0, which looks relevant, but we're not using this style of configuration, right?

comment:145 Changed 23 months ago by broken_symlink

Just a heads up that iPython 0.13.1 is set to be released on Friday. It fixes all the bugs I was having with ipython.parallel.

Changed 23 months ago by jhpalmieri

comment:146 Changed 23 months ago by jhpalmieri

  • Description modified (diff)

We need another patch, since the pager in IPython has moved. One instance of this was fixed in one of the patches, but we need to fix others.

Note the change to sagedoc.py; this needs to be tested.

Last edited 23 months ago by jhpalmieri (previous) (diff)

comment:147 Changed 23 months ago by jason

It seems like you caught all of the instances of the IPython pager:

grout@tiny:~/sage/devel/sage/sage% ack genutils *
interfaces/gap3.py
493:            import IPython.genutils
494:            IPython.genutils.page(helptext)

misc/pager.py
27:        import IPython.genutils
28:        return IPython.genutils.page

misc/sagedoc.py
833:        from IPython.genutils import page

comment:148 Changed 23 months ago by jason

  • Description modified (diff)

comment:149 Changed 23 months ago by jason

Positive review to the sagedoc.py change---search_src didn't work before, but it does work after the change.

comment:150 Changed 23 months ago by jason

I uploaded an IPython 0.13.1 spkg, updating to the latest stable. (the old 0.13 spkg is still in my home directory too).

Changed 23 months ago by jhpalmieri

comment:151 Changed 23 months ago by jhpalmieri

  • Description modified (diff)

trac_12719-move_to_preparser.patch doesn't apply cleanly to 5.4.rc2. Here's a rebased version.

comment:152 follow-up: Changed 23 months ago by jason

  • Description modified (diff)

jhpalmieri: I uploaded a new IPython 0.13.1 spkg that applies https://github.com/ipython/ipython/pull/2403. This means we can delete the 12719-preparse-magic.patch patch, which will solve the %runfile problems, but keep the preparsing for %time, %timeit, and %prun.

I've updated the instructions.

comment:153 Changed 23 months ago by jason

Whew, with all these patches---I can't wait until we move to using git (or even mercurial properly) and we have branches for this sort of thing.

comment:154 follow-up: Changed 23 months ago by jason

The cython function printing issue appears to boil down to those functions being treated as builtin functions, so this line applies: https://github.com/ipython/ipython/blob/master/IPython/lib/pretty.py#L669, rather than this line: https://github.com/ipython/ipython/blob/master/IPython/lib/pretty.py#L671

Suggestions for how to fix this? How do we distinguish between built-in functions and Cython methods?

comment:155 Changed 23 months ago by jason

I've posted to the IPython list about the Cython method printing: http://mail.scipy.org/pipermail/ipython-dev/2012-October/010503.html

comment:156 Changed 23 months ago by jason

  • Work issues changed from ship https://github.com/ipython/ipython/pull/2403 to Cython method printing

Are there any other outstanding issues other than the Cython method printing?

comment:157 in reply to: ↑ 152 Changed 23 months ago by jhpalmieri

Replying to jason:

jhpalmieri: I uploaded a new IPython 0.13.1 spkg that applies https://github.com/ipython/ipython/pull/2403. This means we can delete the 12719-preparse-magic.patch patch, which will solve the %runfile problems, but keep the preparsing for %time, %timeit, and %prun.

Can you think of a way to doctest this? (I haven't tried anything at all, so it might be easy.)

comment:158 Changed 23 months ago by jason

If just putting in a magic %timeit ... doesn't work, then this should work *if* the doctesting framework is using IPython:

sage: get_ipython().run_line_magic('time', '594.factor()')
CPU times: user 0.00 s, sys: 0.00 s, total: 0.00 s
Wall time: 0.00 s
2 * 3^3 * 11

comment:159 Changed 23 months ago by jhpalmieri

  • Description modified (diff)

Hey, let's add another patch to the pile!

Changed 23 months ago by jhpalmieri

comment:160 in reply to: ↑ 154 Changed 23 months ago by jhpalmieri

Replying to jason:

The cython function printing issue appears to boil down to those functions being treated as builtin functions, so this line applies: https://github.com/ipython/ipython/blob/master/IPython/lib/pretty.py#L669, rather than this line: https://github.com/ipython/ipython/blob/master/IPython/lib/pretty.py#L671

Suggestions for how to fix this? How do we distinguish between built-in functions and Cython methods?

I don't know. Even in previous versions of Sage:

sage: type(4.gcd)
<type 'builtin_function_or_method'>

I wonder if changing this would require patching Cython. If we knew where the methods for Cython methods were defined, maybe we could just define a method _repr_pretty_, as described in https://github.com/ipython/ipython/blob/master/IPython/lib/pretty.py#L27. Line 3737-3740 of local/lib/python/site-packages/Cython/Compiler/Nodes.py says

        is_builtin_function_or_method = "PyCFunction_Check(%s)" % func_node_temp
        is_overridden = "(PyCFunction_GET_FUNCTION(%s) != (PyCFunction)%s)" % (
            func_node_temp, self.py_func.entry.func_cname)
        code.putln("if (!%s || %s) {" % (is_builtin_function_or_method, is_overridden))

Is this relevant?

Alternatively, we could patch the file "pretty.py", modifying the definition of _type_pprinters somehow, although I'm not sure how.

Alternatively, we could temporarily accept the new print representation for Cython-defined methods, and create a new ticket for fixing this. It seems like a small issue to me, and maybe it shouldn't hold up the rest of this ticket.

Last edited 23 months ago by jhpalmieri (previous) (diff)

comment:161 Changed 23 months ago by jason

  • Cc robertwb added

Alternatively, we could post to the Cython list and ask them :).

Ccing robertwb (see the comment or two above about Cython function printing, as well as http://mail.scipy.org/pipermail/ipython-dev/2012-October/010503.html, which tries to lay out the issue more concisely).

comment:162 Changed 23 months ago by jhpalmieri

Asking the Cython developers is a great idea. Here's a real hack, but it seems to work: for built-in functions, if the print representation includes "sage.", we use that. (Now that I look at your post to the IPython list, it's just a version of the hack you described there.) This is a patch for IPython/lib/pretty.py:

  • pretty.py

    old new  
    616616 
    617617def _function_pprint(obj, p, cycle): 
    618618    """Base pprint for all functions and builtin functions.""" 
    619     if obj.__module__ in ('__builtin__', 'exceptions') or not obj.__module__: 
    620         name = obj.__name__ 
     619    if obj.__repr__().find('sage.') != -1: 
     620        p.text(obj.__repr__()) 
    621621    else: 
    622         name = obj.__module__ + '.' + obj.__name__ 
    623     p.text('<function %s>' % name) 
     622        if obj.__module__ in ('__builtin__', 'exceptions') or not obj.__module__: 
     623            name = obj.__name__ 
     624        else: 
     625            name = obj.__module__ + '.' + obj.__name__ 
     626        p.text('<function %s>' % name) 
    624627 
    625628 
    626629def _exception_pprint(obj, p, cycle): 

comment:163 Changed 23 months ago by jhpalmieri

On the Cython users mailing list, in which I essentially quoted Jason's message to the IPython list, Bradley Froehle says

There's nothing to be done in Cython here.  As you point out, 
a.seed is a builtin (i.e. compiled) method and type(a.seed) returns
`builtin_function_or_method` correctly.

So I think our options are

  • hack pretty.py
  • wait for a real IPython fix
  • change the doctests to match the current output for now, and open a new ticket to fix this later.

Preferences? Am I missing any good choices?

comment:164 Changed 22 months ago by jhpalmieri

Ping. Any opinions?

comment:165 Changed 22 months ago by jason

Is it hard to change the doctests (how many?). If that's easy, I'd say to change the doctests, especially given that any patch we make will take a long time to make it back into Sage through IPython. I'd veto "wait for a real IPython fix".

Is this the only thing holding us back now?

Last edited 22 months ago by jason (previous) (diff)

comment:166 Changed 22 months ago by jhpalmieri

  • Description modified (diff)

I think there is only one failing doctest:

sage -t  --long -force_lib devel/sage/sage/interfaces/sage0.py
**********************************************************************
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/12719/sage-5.4.rc3/devel/sage-main/sage/interfaces/sage0.py", \
line 489:
    sage: sage0(4).gcd
Expected:
    <built-in method gcd of sage.rings.integer.Integer object at 0x...>
Got:
    <function gcd>
**********************************************************************

and it should be trivial to fix. I also don't know how serious the issue is. I mean, it would be nice if 4.gcd printed more information, but it doesn't seem crucial in any way.

Changed 22 months ago by jhpalmieri

comment:167 Changed 22 months ago by jason

I'd say let's change our doctest, and then work with upstream to get a patch in. When the patch comes down (probably in a few months to a year?), we'll change our doctest again.

comment:168 Changed 22 months ago by jhpalmieri

I changed the doctest. All tests pass for me on sage.math. Since it was already positively reviewed once before, I think we can set it back to that. What do you think?

comment:169 Changed 22 months ago by jason

Yippee!!

Last edited 22 months ago by jason (previous) (diff)

comment:170 Changed 22 months ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:171 Changed 22 months ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-5.5
  • Work issues Cython method printing deleted

comment:172 Changed 22 months ago by jason

jdemeyer -- it would be great if this were merged into one of the 5.5 betas.

comment:173 Changed 22 months ago by jdemeyer

Of course, don't worry.

comment:174 Changed 22 months ago by jdemeyer

  • Description modified (diff)

comment:175 Changed 22 months ago by jdemeyer

  • Dependencies changed from #13459 to #13459, #13211
  • Milestone changed from sage-5.5 to sage-pending

With #13211, I get

applying /release/merger/patches/trac_12719-5.1beta1.patch
patching file sage/interfaces/gap.py
Hunk #2 FAILED at 1065
1 out of 2 hunks FAILED -- saving rejects to file sage/interfaces/gap.py.rej

Note that #13211 isn't certain to be merged (there were some problems before and the fixes haven't been properly tested yet).

comment:176 Changed 22 months ago by jason

I don't think I've ever applied #13211, and I certainly don't have it applied now. I didn't realize it was a dependency (I've just been pasting in the instructions in the description). Can we just remove the dependency? I'm not sure why we even have it.

comment:177 Changed 22 months ago by jdemeyer

My point is that it should be made a dependency and be rebased to #13211.

comment:178 follow-up: Changed 22 months ago by jason

But if #13211's future isn't sure yet, should we add that as a dependency? Granted that the rebase (for either ticket) is a simple 1-2 line fix, but it'd be a shame if this huge work didn't get in because #13211 for some reason didn't pass muster.

You're the release manager, so you make the calls. Do you still want me to rebase to depend on that ticket? Does that hurt our chances of getting into 5.5?

comment:179 in reply to: ↑ 178 Changed 22 months ago by jdemeyer

Replying to jason:

You're the release manager, so you make the calls. Do you still want me to rebase to depend on that ticket? Does that hurt our chances of getting into 5.5?

I will test #13211 and then I'll know better whether or not it will be merged. So let's wait for now.

The best chances for getting this merged is to respond quickly when I ask for it to be rebased.

Changed 22 months ago by jhpalmieri

comment:180 Changed 22 months ago by jhpalmieri

  • Description modified (diff)

Here's a patch rebased to #13211. Let's keep the old patch, too, just in case.

comment:181 Changed 22 months ago by jdemeyer

  • Dependencies changed from #13459, #13211 to #13459, #9191
  • Description modified (diff)
  • Status changed from positive_review to needs_work

Changed 22 months ago by jdemeyer

comment:182 Changed 22 months ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:183 Changed 22 months ago by jhpalmieri

  • Description modified (diff)

I think this looks okay. I'm building and running doctests one more time to make sure.

comment:184 Changed 22 months ago by jhpalmieri

  • Status changed from needs_review to positive_review

Changed 22 months ago by jhpalmieri

comment:185 Changed 22 months ago by jhpalmieri

  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:186 Changed 22 months ago by jhpalmieri

  • Status changed from needs_work to needs_review

comment:187 Changed 22 months ago by jdemeyer

  • Reviewers changed from Volker Braun, Mike Hansen, Jason Grout to Volker Braun, Mike Hansen, Jason Grout, Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:188 Changed 22 months ago by jdemeyer

  • Status changed from positive_review to needs_work

trac_12719-scripts.patch is missing a commit message.

12719-5.4rc1.patch is missing a user name in the patch file.

trac_12719-cellmagics013.patch has a commit message which is all on one line and too long. You should make sure that the lines in commit messages are not too long, but the first line should still be descriptive by itself, since that shows up in hg log.

positive review to trac_12719-hgignore.patch

comment:189 Changed 22 months ago by jdemeyer

In SageInteractiveShell.system_raw, does libraries refer to a global or is that variable simply unused? Since false isn't guaranteed to return exit status 1 (on Solaris, it returns 255, POSIX specifies it must be non-zero), the doctest must be changed to something more portable. I also don't see this function discussed on #975.

So, I suggest the following patch:

  • sage/misc/interpreter.py

    diff --git a/sage/misc/interpreter.py b/sage/misc/interpreter.py
    a b class SageInteractiveShell(TerminalInter 
    161161 
    162162    def system_raw(self, cmd): 
    163163        """ 
    164         Adjust the libraries before calling system commands.  See Trac 
    165         #975 for a discussion of this function. 
     164        Run a system command. 
    166165 
    167166        EXAMPLES:: 
    168167 
    169168            sage: from sage.misc.interpreter import get_test_shell 
    170169            sage: shell = get_test_shell() 
    171170            sage: shell.system_raw('false') 
    172             sage: shell.user_ns['_exit_code'] 
    173             256 
     171            sage: status = shell.user_ns['_exit_code'] 
     172            sage: os.WIFEXITED(status) and os.WEXITSTATUS(status) != 0 
     173            True 
    174174        """ 
    175         sage_commands = os.listdir(os.environ['SAGE_ROOT']+"/local/bin/") 
    176         DARWIN_SYSTEM = os.uname()[0]=='Darwin' 
    177         if cmd in sage_commands: 
    178             return super(SageInteractiveShell, self).system_raw(cmd) 
    179         else: 
    180             libraries = 'LD_LIBRARY_PATH=$$SAGE_ORIG_LD_LIBRARY_PATH;' 
    181             if DARWIN_SYSTEM: 
    182                 libraries += 'DYLD_LIBRARY_PATH=$$SAGE_ORIG_DYLD_LIBRARY_PATH;' 
    183             return super(SageInteractiveShell, self).system_raw(cmd) 
     175        return super(SageInteractiveShell, self).system_raw(cmd) 
    184176 
    185177    def ask_exit(self): 
    186178        """ 
Last edited 22 months ago by jdemeyer (previous) (diff)

comment:190 Changed 22 months ago by jason

  • Description modified (diff)

I've uploaded a patch to the system command. The point is that we need to reset the original system libraries if we run a non-sage command.

Changed 22 months ago by jason

Apply to sage repository

Changed 22 months ago by jason

rebased to 5.4rc1

Changed 22 months ago by jason

comment:191 Changed 22 months ago by jason

  • Status changed from needs_work to needs_review

Okay, I think I've taken care of the outstanding issues with the patches.

comment:192 Changed 22 months ago by jason

I updated the system patch to also include a doctest checking to see if a system command is running with the Sage LD_LIBRARY_PATH.

comment:193 Changed 22 months ago by jdemeyer

You should also doctest system_raw working succesfully.

I think you can replace

os.file.isfile(FILE)

by

os.access(FILE, os.X_OK):

since you want to check for an executable file.

Also, I assume that "cmd" uses shell syntax? Then you're doing it wrong. Correct shell syntax is

LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"; export LD_LIBRARY_PATH

comment:194 Changed 22 months ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:195 Changed 22 months ago by jhpalmieri

I'm not an expert on bash syntax (I assume this is using bash), but perhaps some (or all?) of these would work:

LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"; export LD_LIBRARY_PATH; ...
export LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"; ...
env LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"  ...   # note no semicolon
LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"  ...       # ??

comment:196 Changed 22 months ago by jdemeyer

I don't know whether we can assume bash, I would go for Posix /bin/sh compatible.

For example

export LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH";

is bash-specific and I'm not sure about

LD_LIBRARY_PATH="$SAGE_ORIG_LD_LIBRARY_PATH"  ...

comment:197 Changed 22 months ago by jason

  • Status changed from needs_work to needs_review

I hope this redo is good! Thanks for the tips.

comment:198 Changed 22 months ago by jdemeyer

  • Status changed from needs_review to needs_work

You should replace $$ by $.

comment:199 Changed 22 months ago by jdemeyer

And please also doctest a non-Sage command working successfully. For example

system_raw("true")

Changed 22 months ago by jason

apply to sage repository

comment:200 Changed 22 months ago by jason

  • Status changed from needs_work to needs_review

comment:201 Changed 22 months ago by jhpalmieri

All of the patches apply cleanly (on top of 5.5.beta0 + #9191), and the new 12719-system.patch looks okay to me. Jeroen, what do you think?

I have one mild complaint: if I do

sage: note[TAB]

it doesn't complete to "notebook" anymore, saying that "%notebook" is also a valid completion. Would it be possible, here or on a followup ticket, to rename the "%notebook" magic command to "%ipython_notebook" or something similar?

comment:202 Changed 22 months ago by jdemeyer

  • Status changed from needs_review to positive_review

Looks fine. As for the TAB-completion: a different ticket please.

comment:203 Changed 22 months ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.5

comment:204 Changed 22 months ago by jdemeyer

Upgrading from sage-4.5 fails with a trial version of sage-5.5.beta2 and this is the only ticket I can think of that could cause this problem. The error shows up in Twisted (part of sagenb now):

Installed /release/buildbot/sage/sage-1/sage_upgrade_4.5/build/sage-5.5.beta2/local/lib/python2.7/site-packages/Twisted-12.1.0-py2.7-linux-x86_64.egg
Processing dependencies for Twisted==12.1.0
Searching for zope.interface

Link to http://pypi.python.org/simple/zope.interface/ ***BLOCKED*** by --allow-hosts

Couldn't find index page for 'zope.interface' (maybe misspelled?)
Scanning index of all packages (this may take a while)

Link to http://pypi.python.org/simple/ ***BLOCKED*** by --allow-hosts

No local packages or download links found for zope.interface
error: Could not find suitable distribution for Requirement.parse('zope.interface')
Error installing Twisted-12.1.0.tar.bz2 !

comment:205 Changed 22 months ago by jdemeyer

  • Milestone changed from sage-5.5 to sage-5.6

Whether or not IPython causes the zope.interface problem, I won't have time anyway to test it...

comment:206 Changed 22 months ago by jason

This seems like it has to be a sagenb problem (that we aren't requiring zope.interface, and thus packaging zope.interface, in our sagenb prereqs).

Upgrading from sage 4.5? I'm almost sure that it's the sagenb upgrade at #13121, not this ticket.

comment:207 Changed 22 months ago by jason

Okay, here's what makes sense: the old IPython required zope.interface, so it was always installed when we installed the new notebook, so we never noticed that we needed it for the sage notebook. The new IPython no longer requires zope.interface, so when the notebook at #13121 checks for it, it isn't installed so sagenb tries to install it.

The correct fix is to make sagenb require and package zope.interface, like it should have before now, not to change this ticket.

comment:208 Changed 22 months ago by jason

  • Dependencies changed from #13459, #9191 to #13459, #9191, #13717

I uploaded a new sagenb spkg at #13717

comment:209 Changed 22 months ago by jdemeyer

  • Status changed from positive_review to needs_work

Another upgrade issue:

sage_root-5.5.beta2
Machine:
Darwin bsd.math.washington.edu 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun  7 16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64 x86_64
Deleting directories from past builds of previous/current versions of sage_root-5.5.beta2
Extracting package /Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/spkg/standard/sage_root-5.5.beta2.spkg ...
-rw-r--r--  1 buildbot  staff  323619 Nov 16 04:38 /Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/spkg/standard/sage_root-5.5.beta2.spkg
Finished extraction
****************************************************
Host system
uname -a:
Darwin bsd.math.washington.edu 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun  7 16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64 x86_64
****************************************************
****************************************************
CC Version
gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local/libexec/gcc/x86_64-apple-darwin10.8.0/4.6.3/lto-wrapper
Target: x86_64-apple-darwin10.8.0
Configured with: ../src/configure --prefix=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local --with-local-prefix=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local --with-gmp=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local --with-mpfr=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local --with-mpc=/Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2/local --with-system-zlib --disable-multilib  
Thread model: posix
gcc version 4.6.3 (GCC) 
****************************************************
cp: ipython: No such file or directory
Root repo: Error copying files from Sage root repo to /Users/buildbot/build/sage/bsd-1/bsd_upgrade_4.6.2/build/sage-5.5.beta2

The file spkg/root-spkg-install should be fixed.

comment:210 Changed 22 months ago by jdemeyer

Jason: that makes sense.

Changed 22 months ago by jhpalmieri

root repo

comment:211 Changed 22 months ago by jhpalmieri

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I think this patch will fix the problem.

comment:212 Changed 21 months ago by jason

Here's another problem highlighted by https://github.com/sagemath/sagecell/issues/378:

"""string
"""; type(1)

does no preparsing on the second line because IPython doesn't do transformations for the second line (since it's inside quotes). IPython admits that this is a hackish and fragile strategy, and they are working on the issue. For example, they are implementing AST transformers and stateful line transformers. However, for now, we should run at least our preparser on every line of input, since we *do* keep track of state, so we need to be preparsing that second line.

I have a patch coming up shortly.

Changed 21 months ago by jason

apply to sage library

comment:213 Changed 21 months ago by jason

  • Description modified (diff)

comment:214 Changed 21 months ago by jhpalmieri

Does this most recent patch need a doctest?

comment:215 Changed 21 months ago by jhpalmieri

By the way, this passes all doctests. I also upgraded Sage 5.2 successfully from a source distribution prepared using the patches here.

comment:216 Changed 20 months ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-5.6 to sage-pending

Changed 20 months ago by vbraun

Updated patch

comment:217 Changed 20 months ago by vbraun

  • Description modified (diff)

I've rediffed trac_12719-scripts-vb.patch to account for pure whitespace changes in #13899 (merged in sage-5.6.beta3)

comment:218 Changed 20 months ago by vbraun

  • Description modified (diff)

comment:219 Changed 20 months ago by vbraun

  • Description modified (diff)
  • Status changed from needs_review to positive_review

Works for me and as far as I can tell all remaining issues have been addressed.

Does not fix #11223, %bg is not implemented in ipython-0.13.1.

Changed 20 months ago by jdemeyer

Rediffed patch

comment:220 Changed 20 months ago by jdemeyer

  • Dependencies changed from #13459, #9191, #13717 to #13459, #9191, #13717, #13963

comment:221 Changed 20 months ago by jdemeyer

  • Status changed from positive_review to needs_work

This seems to cause some problems on OpenSolaris.

First a doctest time-out:

Trying:
    sage0.eval('2+2')###line 320:_sage_    sage: sage0.eval('2+2')
Expecting:
    '4'
[...hangs...]

Interrupting this and restarting Sage gives:

buildbot@hawk:~/build/sage/hawk-1/hawk_suncc/build/sage$ ./sage
----------------------------------------------------------------------
| Sage Version 5.7.beta0, Release Date: 2013-01-20                   |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************

**********************************************************************

Oops, Sage crashed. We do our best to make it stable, but...

A crash report was automatically generated with the following information:
  - A verbatim copy of the crash traceback.
  - A copy of your input history during this session.
  - Data on your current Sage configuration.

It was left in the file named:
        '/export/home/buildbot/.sage/ipython-new/Sage_crash_report.txt'
If you can email this file to the developers, the information in it will help
them in understanding and correcting the problem.

You can mail it to: sage-support at sage-support@googlegroups.com
with the subject 'Sage Crash Report'.

If you want to do it now, the following command will work (under Unix):
mail -s 'Sage Crash Report' sage-support@googlegroups.com < /export/home/buildbot/.sage/ipython-new/Sage_crash_report.txt

To ensure accurate tracking of this issue, please file a report about it at:
http://trac.sagemath.org/sage_trac

Hit <Enter> to quit (your terminal may close)

Removing $HOME/.sage fixed that.

comment:222 follow-up: Changed 20 months ago by vbraun

Can we have a beta0 with this ticket? Its a bit ridiculous to have to use a script to apply all these patches.

Also, .sage/ipython-new? What are we going to do the next time the configuration file format changes, .sage/ipython-even-newer?

comment:223 in reply to: ↑ 222 Changed 20 months ago by jdemeyer

Replying to vbraun:

Can we have a beta0 with this ticket?

See http://sage.math.washington.edu/home/release/sage-5.7.beta1

comment:224 Changed 20 months ago by jason

Volker, you're right that the config file directory is still a bit clunky. On the one hand, I didn't want to put the version number in there so that it changes every time we upgrade IPython. On the other hand, calling something 'new' doesn't scale.

What if we put the version number in there like this: ipython-0.12-or-later? That's weird too, but it also means that people don't have to keep moving their configuration files if IPython upgrades, but the config format stays the same.

comment:225 Changed 20 months ago by vbraun

I think we are smart enough to figure out that ipython-0.12 is the directory for configuration files that were introduced with iPython 0.12 and that you might run a later (but still compatible) version.

comment:226 Changed 20 months ago by jason

But IIRC, Matplotlib uses a per-version config directory inside of .sage, so having two conventions could be confusing.

comment:227 Changed 20 months ago by jason

What about the name IPython-0.12+?

comment:228 Changed 20 months ago by vbraun

I don't really care about the name as long as it has the version somewhere in there. Personally I find anything extra just confusing but feel free to pick what you like.

comment:229 Changed 20 months ago by vbraun

Jeroen: can you attach the /export/home/buildbot/.sage/ipython-new/Sage_crash_report.txt

comment:230 Changed 20 months ago by vbraun

I tested this on OpenIndiana (OpenSolaris clone) and I don't see the timeout that Jeroen has. I get occasional non-reproducible timeouts on OI though, it might be something generally wonky with their PTYs. Remind me again why something that nobody is using or has access to is a primary platform?

Last edited 20 months ago by vbraun (previous) (diff)

comment:231 Changed 20 months ago by jhpalmieri

I see a crash on David Kirkby's machine hawk. Crash log posted here.

Last edited 20 months ago by jhpalmieri (previous) (diff)

comment:232 Changed 20 months ago by vbraun

Sane catching of history database errors has been fixed upstream at https://github.com/ipython/ipython/commit/d347c347bab915df69d499521b4876d3c9b168b9, but not in a released version.

See also: https://github.com/ipython/ipython/issues/2097

Last edited 20 months ago by vbraun (previous) (diff)

comment:233 Changed 20 months ago by vbraun

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I've backported the changes to history.py, this should fix the startup failure.

Last edited 20 months ago by vbraun (previous) (diff)

comment:234 Changed 20 months ago by jason

Keeping track, I guess there's one more thing to do: change the config directory back to .sage/ipython-0.12 in trac_12719-ipythondir013.patch

Last edited 20 months ago by jason (previous) (diff)

Changed 20 months ago by jason

rebased to 5.4rc1

comment:235 Changed 20 months ago by jason

Okay, I changed the ipythondir013.patch to use the config directory ipython-0.12. So that needs review too.

comment:236 Changed 20 months ago by jason

  • Work issues set to history-related crash, config directory name

Updating work issues with the things that need review.

comment:237 Changed 20 months ago by vbraun

  • Status changed from needs_review to positive_review
  • Work issues history-related crash, config directory name deleted

I'm fine with your patch for the directory name. Any expect interface on OpenIndiana occasionally crashes (maybe 1 in 20), but I've verified we are now not permanently stuck with a corrupted history file any more.

comment:238 follow-up: Changed 20 months ago by jhpalmieri

On hawk, I see a doctest failure, but I'm not sure how it could be related to this ticket:

sage -t  --long -force_lib devel/sage/sage/tests/french_book/mpoly.py                     
**********************************************************************                    
File "/export/home/palmieri/testing/clean/sage-5.7.beta1/devel/sage-main/sage/tests/frenc\
h_book/mpoly.py", line 373:                                                               
    sage: J.variety(RDF)                                                                  
Expected:                                                                                 
    [{y: 396340.89..., x: 26.61226...}]                                                   
Got:                                                                                      
    [{y: 396340.890167, x: -46.7888621592}]                                               
**********************************************************************                    
1 items had failures:                                                                     
   1 of 151 in __main__.example_0                                                         
***Test Failed*** 1 failures.                       

I also see a time out:

sage -t  --long -force_lib devel/sage/sage/interfaces/sage0.py                            
*** *** Error: TIMED OUT! PROCESS KILLED! *** ***                                         
                                                                                          
         [1800.2 s] 

which perhaps is related to this ticket.

comment:239 in reply to: ↑ 238 Changed 20 months ago by jdemeyer

Replying to jhpalmieri:

sage -t --long -force_lib devel/sage/sage/tests/french_book/mpoly.py

Not related to this ticket.

sage -t  --long -force_lib devel/sage/sage/interfaces/sage0.py                            
*** *** Error: TIMED OUT! PROCESS KILLED! *** ***                                         
                                                                                          
         [1800.2 s] 

This is indeed related to this ticket, see 221. But I haven't tried with the latest version of the spkg.

Last edited 20 months ago by jdemeyer (previous) (diff)

comment:240 Changed 20 months ago by jdemeyer

  • Status changed from positive_review to needs_work

Patches trac_12719_ROOT_configuration_files.patch and trac_12719-ipythondir013.patch combined and rebased. And moved the setting of IPYTHONDIR to sage-env where it belongs.

Changed 20 months ago by jdemeyer

Rebased to sage-5.7.beta0

comment:241 Changed 20 months ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:242 Changed 20 months ago by vbraun

  • Status changed from needs_review to positive_review

Sounds good to me.

comment:243 Changed 20 months ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.7

comment:244 Changed 20 months ago by jason

Volker and Jeroen: thanks for both of you working quickly on the last remaining issues!

comment:245 Changed 20 months ago by jhpalmieri

  • Description modified (diff)

comment:246 follow-up: Changed 20 months ago by jhpalmieri

One patch doesn't apply cleanly to 5.6.rc1:

adding trac_12719-rebased-to-13211-vb.patch to series file
applying trac_12719-rebased-to-13211-vb.patch
patching file sage/interfaces/gap.py
Hunk #1 FAILED at 177
1 out of 2 hunks FAILED -- saving rejects to file sage/interfaces/gap.py.rej

I think that the hang on hawk is gone with the latest versions, but I'm testing again to make sure.

comment:247 in reply to: ↑ 246 Changed 20 months ago by vbraun

Replying to jhpalmieri:

One patch doesn't apply cleanly to 5.6.rc1:

You might be missing a dependency since the patch is in sage-5.7.beta1...

comment:248 Changed 20 months ago by jhpalmieri

  • Status changed from positive_review to needs_work

I didn't think that 5.7.beta0 had been released. Am I supposed to use the unofficial version of it?

By the way, since the file ipy_profile_sage.py has been deleted, any mention of it should also be removed from sage-make_devel_packages and sage-spkg-install.

comment:249 Changed 20 months ago by jason

Can someone test something really quick with the official version of the ticket, if it's convenient?

Do:

1; 2

Does it print out both 1 and 2 in both the command line and the notebook? Right now, aleph only prints out 2, and I've narrowed it down to how IPython handles the 'print out the last expression' (IPython prints out the last expression, while Sage prints out the last physical line).

comment:250 Changed 20 months ago by vbraun

John, see comment:223. Since Jeroen linked to it I'd say its official ;-)

Jason, works for me (this is with the sagenb update but it shouldn't matter):

[vbraun@volker-desktop sage-5.7.beta1]$ ./sage
----------------------------------------------------------------------
| Sage Version 5.7.beta1, Release Date: 2013-01-21                   |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
sage:  1; 2
1
2

Changed 20 months ago by vbraun

Initial patch

comment:251 Changed 20 months ago by vbraun

  • Description modified (diff)
  • Status changed from needs_work to positive_review

I've removed the last occurrences from ipy_profile_sage.py.

comment:252 follow-up: Changed 20 months ago by jason

Volker: and in the sage notebook, it does the same thing? I think it should, but I just want to make sure.

comment:253 in reply to: ↑ 252 Changed 20 months ago by vbraun

Replying to jason:

Volker: and in the sage notebook, it does the same thing?

Yes, same (correct) output in the notebook (with the sagenb-0.10.4 spkg).

comment:254 Changed 20 months ago by jdemeyer

Could somebody who knows this ticket please look at #12167 and #12926 to check whether they might be closed?

comment:255 Changed 20 months ago by jason

I think both of those can be closed. The new config system is set up with IPYTHONDIR, which is in DOTSAGE. sage --ipython should use that now.

comment:256 Changed 19 months ago by novoselt

Hello,

I just want to confirm that this is expected with this ticket:

----------------------------------------------------------------------
| Sage Version 5.6, Release Date: 2013-01-21                         |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
sage: exit
'Use Ctrl-D (i.e. EOF), %Exit, or %Quit to exit without confirmation.'
sage: 

Was "just exit for exit" a feature of old IPython or Sage?

Changed 19 months ago by jdemeyer

comment:257 Changed 19 months ago by jdemeyer

  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:258 Changed 19 months ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:259 Changed 19 months ago by vbraun

I don't know whats up with the overly attached Python interpreter but at least Ctrl-D works. Imho exit should just do that but that can be done (and doctested) on another ticket.

Jeroen's patch looks good to me.

comment:260 Changed 19 months ago by vbraun

  • Status changed from needs_review to positive_review

comment:261 Changed 19 months ago by vbraun

  • Description modified (diff)

comment:262 Changed 19 months ago by jdemeyer

The timeout on hawk of 221 still exists, I'll have a look.

comment:263 Changed 19 months ago by jdemeyer

This is the cause of the crash on hawk, it still has to do with the history database: http://boxen.math.washington.edu/home/buildbot/crashlogs/hawk_ipython_crash.log

comment:264 Changed 19 months ago by jdemeyer

  • Status changed from positive_review to needs_work

We should probably disable the command history for the sage0() interface, but I don't know how one can do that.

comment:265 Changed 19 months ago by vbraun

Can be done in IPython with

ipython --HistoryManager.hist_file=:memory:

comment:266 follow-up: Changed 19 months ago by vbraun

How about we run sage0 with --nodotsage?

comment:267 in reply to: ↑ 266 Changed 19 months ago by jdemeyer

Replying to vbraun:

How about we run sage0 with --nodotsage?

No, having access to the IPython crash log would be good.

I'm preparing a patch, hang on...

Changed 19 months ago by jdemeyer

comment:268 Changed 19 months ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:269 Changed 19 months ago by jdemeyer

  • Authors changed from Mike Hansen, Volker Braun, Jason Grout to Mike Hansen, Volker Braun, Jason Grout, Jeroen Demeyer

comment:270 Changed 19 months ago by vbraun

I've reported the issue upstream: https://github.com/ipython/ipython/issues/2845

comment:271 Changed 19 months ago by vbraun

  • Status changed from needs_review to positive_review

Sounds good to me. I've verified that this disables caching. I don't have a harddisk that is slow enough to trigger the locking issue ;-)

comment:272 Changed 19 months ago by jdemeyer

On sage.math, over NFS it's fairly easy to reproduce.

comment:273 Changed 19 months ago by jdemeyer

  • Merged in set to sage-5.7.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:274 Changed 19 months ago by jdemeyer

John Cremona reports on sage-release:

jec@fermat%./sage
----------------------------------------------------------------------
| Sage Version 5.7.beta1, Release Date: 2013-01-26                   |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
/home/jec/sage-5.7.beta1/local/lib/python2.7/site-packages/IPython/utils/py3compat.pyc
in execfile(fname, *where)
    176             else:
    177                 filename = fname
--> 178             __builtin__.execfile(filename, *where)

/home/jec/.sage/init.sage in <module>()
----> 1 load /home/jec/sage/ecdb.py

NameError: name 'load' is not defined

comment:275 Changed 19 months ago by jdemeyer

Volker Braun reports:

/mnt/storage2TB/patchbot/Sage/sage-5.7.beta1/local/lib/python2.7/site-packages/sage/misc/sage_extension.pyc in print_branch(self=<sage.misc.sage_extension.SageCustomizations object>)
    423     def set_quit_hook(self):
    424         """
    425         Set the exit hook to cleanly exit Sage.  This does not work in all cases right now.
    426         """
    427         def quit(shell):
    428             import sage
    429             sage.all.quit_sage()
    430         self.shell.set_hook('shutdown_hook', quit)
    431 
    432     def print_branch(self):
    433         """
    434         Print the branch we are on currently
    435         """
    436         from sage.misc.misc import branch_current_hg_notice, branch_current_hg
    437         branch = branch_current_hg_notice(branch_current_hg())
--> 438         if branch and not self.shell.test_shell:
        branch = 'Loading Sage library. Current Mercurial branch is: 0'
        self.shell.test_shell = undefined
    439             print branch
    440 
    441     def init_environment(self):
    442         """
    443         Set up Sage command-line environment
    444         """
    445         try:
    446             self.shell.run_cell('from sage.all import Integer, RealNumber')
    447         except Exception:
    448             import traceback
    449             print "Error importing the Sage library"
    450             traceback.print_exc()
    451             print
    452             print "To debug this, you can run:"
    453             print 'sage -ipython -i -c "import sage.all"'

AttributeError: 'SageInteractiveShell' object has no attribute 'test_shell'

comment:276 Changed 19 months ago by vbraun

I've created #14024 to collect any patches that we need on top of Sage-5.7.beta1

comment:277 Changed 19 months ago by jdemeyer

  • Description modified (diff)

comment:278 Changed 19 months ago by jdemeyer

Please have a look at #14042, another follow-up.

comment:279 Changed 19 months ago by jdemeyer

There is still this problem:

jec@fermat%./sage
----------------------------------------------------------------------
| Sage Version 5.7.beta1, Release Date: 2013-01-26                   |
| Type "notebook()" for the browser-based notebook interface.        |
| Type "help()" for help.                                            |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
/home/jec/sage-5.7.beta1/local/lib/python2.7/site-packages/IPython/utils/py3compat.pyc
in execfile(fname, *where)
    176             else:
    177                 filename = fname
--> 178             __builtin__.execfile(filename, *where)

/home/jec/.sage/init.sage in <module>()
----> 1 load /home/jec/sage/ecdb.py

NameError: name 'load' is not defined

comment:280 Changed 19 months ago by jason

#14066 was opened to deal with all remaining issues.

comment:281 Changed 18 months ago by jhpalmieri

How are users supposed to configure Sage's IPython now? See sage-support. Right now, it looks like IPython's config files (either .sage/ipython-0.12/profile_default/ipython_config.py or .sage/ipython-0.12/profile_sage/ipython_config.py) are not loaded when Sage initializes its terminal. A possible solution:

  • sage/misc/interpreter.py

    diff --git a/sage/misc/interpreter.py b/sage/misc/interpreter.py
    a b  
    672672 
    673673    def __init__(self, **kwargs): 
    674674        # Overwrite the default Sage configuration with the user's. 
     675        from IPython.frontend.terminal.ipapp import load_default_config 
    675676        new_config = Config() 
    676677        new_config._merge(DEFAULT_SAGE_CONFIG) 
    677678        new_config._merge(kwargs.get('config', Config())) 
     679        new_config._merge(load_default_config()) 
    678680        kwargs['config']=new_config 
    679681        super(SageTerminalApp, self).__init__(**kwargs) 
    680682 

Then .sage/ipython-0.12/profile_default/ipython_config.py will be read and merged with Sage's configuration. Is this the right solution? Should I open a ticket? Or am I missing something?

comment:282 Changed 18 months ago by vbraun

Looks good to me

comment:283 Changed 18 months ago by jhpalmieri

See #14188.

Note: See TracTickets for help on using tickets.