Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#14713 closed enhancement (fixed)

Update to IPython 1.2.1

Reported by: Jason Grout Owned by: Jeroen Demeyer
Priority: major Milestone: sage-6.2
Component: packages: standard Keywords:
Cc: David Roe Merged in:
Authors: Jason Grout Reviewers: William Stein
Report Upstream: N/A Work issues:
Branch: 0fb988d (Commits, GitHub, GitLab) Commit:
Dependencies: #14810 Stopgaps:

Status badges

Description (last modified by R. Andrew Ohana)

IPython 1.2.1 has been released. Changes in IPython resulted in massive cleanup of the custom Sage transformations.

Upstream tarball: https://pypi.python.org/packages/source/i/ipython/ipython-1.2.1.tar.gz

Attachments (2)

new_transformers.patch (16.1 KB) - added by Jason Grout 9 years ago.
trac_14713_new_transformers.patch (18.8 KB) - added by Volker Braun 9 years ago.
Updated patch

Download all attachments as: .zip

Change History (46)

Changed 9 years ago by Jason Grout

Attachment: new_transformers.patch added

comment:1 Changed 9 years ago by Volker Braun

Description: modified (diff)

comment:2 Changed 9 years ago by Volker Braun

Dependencies: #14810

Changed 9 years ago by Volker Braun

Updated patch

comment:3 Changed 9 years ago by Volker Braun

Cc: David Roe added

Everything works except a failure in sage/doctest/forker.py when running the doctest through the sage0 interface. The pexpect log contains the traceback

MultipleInstanceError                     Traceback (most recent call last)
<ipython-input-12-54a61f46e9ab> in <module>()
----> 1 DTR.run(DT, clear_globs=False)

/home/vbraun/opt/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/doctest/forker.pyc in run(self, test, compileflags, out, clear_globs)
    623         self.no_failure_yet = True
    624         try:
--> 625             return self._run(test, compileflags, out)
    626         finally:
    627             self._fakeout.stop_spoofing()

/home/vbraun/opt/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/doctest/forker.pyc in _run(self, test, compileflags, out)
    535             elif outcome is FAILURE:
    536                 if not quiet:
--> 537                     self.report_failure(out, test, example, got, test.globs)
    538                 failures += 1
    539             elif outcome is BOOM:

/home/vbraun/opt/sage-5.11.rc0/local/lib/python2.7/site-packages/sage/doctest/forker.pyc in report_failure(self, out, test, example, got, globs)
   1112                     prompt_config.in_template = 'debug: '
   1113                     prompt_config.in2_template = '.....: '
-> 1114                     embed(config=cfg, banner1='', user_ns=dict(globs))
   1115                 except KeyboardInterrupt:a

   1116                     # Assume this is a *real* interrupt. We need to

/home/vbraun/opt/sage-5.11.rc0/local/lib/python2.7/site-packages/IPython/terminal/embed.pyc in embed(**kwargs)
    298         config.InteractiveShellEmbed = config.TerminalInteractiveShell
    299         kwargs['config'] = config
--> 300     shell = InteractiveShellEmbed.instance(**kwargs)
    301     shell(header=header, stack_depth=2, compile_flags=compile_flags)

/home/vbraun/opt/sage-5.11.rc0/local/lib/python2.7/site-packages/IPython/config/configurable.pyc in instance(cls, *args, **kwargs)
    358             raise MultipleInstanceError(
    359                 'Multiple incompatible subclass instances of '
--> 360                 '%s are being created.' % cls.__name__
    361             )
    362 

MultipleInstanceError: Multiple incompatible subclass instances of InteractiveShellEmbed are being created.

comment:4 Changed 9 years ago by Jason Grout

I've been continuing work on this in my ipython-1.0 branch on github: https://github.com/jasongrout/sage/tree/ipython-1.0

I ran into some changes in IPython system_raw exit code that caused us problems. I posted about it here: http://mail.scipy.org/pipermail/ipython-dev/2013-August/012102.html But IPython 1.0 is released without this being resolved, so I think we ought to just update our doctests that are failing because of it.

I am also doing some experimental work on "string decorators" on that branch, inspired by what William did in sage cloud. However, it's still at a preview stage. I can turn it off by default, though, and we can continue to work on it.

comment:5 Changed 9 years ago by Volker Braun

The exit code thing has already been resolved in #14810

comment:6 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:7 Changed 9 years ago by William Stein

+1 to this. I'm using ipython 1.0 with the patches from the git repo for cloud.sagemath very soon. Here's how:

cd devel/sage/
wget https://github.com/jasongrout/sage/commit/fdbe79ef7ed0ca0fa6c712c4c580ba34de1a1166.patch
wget https://github.com/jasongrout/sage/commit/c1ad5805558b15694d783bbb5c77296f2d492b2e.patch
patch --ignore-whitespace -p2 < fdbe79ef7ed0ca0fa6c712c4c580ba34de1a1166.patch
patch --ignore-whitespace -p2 < c1ad5805558b15694d783bbb5c77296f2d492b2e.patch
cd ../..
./sage -b
rm -rf local/lib/python/site-packages/IPython* local/lib/python/site-package/ipython*
./sage -sh
easy_install ipython

They work.

For convenience, here is a single hg patch:

http://wstein.org/home/wstein/tmp/trac-14713.patch

Last edited 9 years ago by William Stein (previous) (diff)

comment:8 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:9 Changed 9 years ago by Nicolas M. Thiéry

Just for the record: some of my students often get hit by some bracktraces not being printed out properly when using %attach. This looks like https://github.com/ipython/ipython/issues/2512.

Nothing critical, but they will appreciate the upgrade :-)

Thanks!

comment:10 Changed 9 years ago by Jason Grout

2.0 will probably be released within the month or so. I'm tracking the changes on IPython master so it will be an easy upgrade.

comment:11 Changed 9 years ago by Nicolas M. Thiéry

Nice. Thanks for the update!

comment:12 Changed 9 years ago by Jason Grout

Sure, no problem. I'm tracking the changes because I'm running (close to) IPython master for the Sage cell server. My current patches are here: https://github.com/jasongrout/sage/commits/sagecell (up through change d662bf2655dcdc04100a8f29ccb88b790377b46a or so)

comment:13 Changed 9 years ago by R. Andrew Ohana

Branch: u/ohanar/ipython-upgrade
Commit: 7293ef21fc691d367bf3e1dc663565097179040a
Description: modified (diff)
Summary: Update to IPython 1.0Update to IPython 1.2.0

I see no reason to wait for IPython 2.0. There are still some issues that stem from the issue found in #14961.

comment:14 Changed 9 years ago by git

Commit: 7293ef21fc691d367bf3e1dc663565097179040aa86c939634000fc553763b11e07ef1e8acb7e9e8

Branch pushed to git repo; I updated commit sha1. New commits:

a86c939really fix instance creation of InteractiveShellEmbed

comment:15 Changed 9 years ago by git

Commit: a86c939634000fc553763b11e07ef1e8acb7e9e82ffb13cd82cc62d86e2a657ad0f5151a56a80e8c

Branch pushed to git repo; I updated commit sha1. New commits:

24ea892dynamically set IPYTHONDIR based on the current version of ipython
2ffb13cbackport IPython pull request #4504

comment:16 Changed 9 years ago by R. Andrew Ohana

There is a single doctest that fails (in src/sage/misc/interpreter.py), but it doesn't match the behavior of running the test from the sage command line.

Also, we need some doctests for sage.misc.interpreter.SagePreparseTransformer.reset. Since I don't know the preparser code (and the reset keyword is documented as "a boolean"), it would be great if someone knowledgeable about this could fill in something here.

@jason I deleted the note about the prun_magics.patch since you deleted it in https://github.com/jasongrout/sage/commit/d662bf2655dcdc04100a8f29ccb88b790377b46a. Do we still need this?

comment:17 in reply to:  13 Changed 9 years ago by William Stein

Replying to ohanar:

I see no reason to wait for IPython 2.0. There are still some issues that stem from the issue found in #14961.

I strongly agree with not waiting for IPython 2.0. They also make some controversial modal changes to the IPython notebook, and I want to see how those shake out in wider testing before switching (i.e., it might be best to wait for 2.1).

comment:18 Changed 9 years ago by William Stein

How is this supposed to actually work?

Attempting to download package ipython-1.2.0
>>> Trying to download http://www.sagemath.org/packages/upstream/ipython/ipython-1.2.0.tar.gz
[Traceback (most recent call last):
  File "<stdin>", line 35, in <module>
  File "/usr/local/sage/sage-6.2/local/lib/python/urllib.py", line 240, in retrieve
    fp = self.open(url, data)
  File "/usr/local/sage/sage-6.2/local/lib/python/urllib.py", line 208, in open
    return getattr(self, name)(url)
  File "/usr/local/sage/sage-6.2/local/lib/python/urllib.py", line 359, in open_http
    return self.http_error(url, fp, errcode, errmsg, headers)
  File "/usr/local/sage/sage-6.2/local/lib/python/urllib.py", line 376, in http_error
    return self.http_error_default(url, fp, errcode, errmsg, headers)
  File "<stdin>", line 17, in http_error_default
IOError: [Errno 404] Not Found: '//www.sagemath.org/packages/upstream/ipython/ipython-1.2.0.tar.gz'
Error: failed to download package ipython-1.2.0
make[2]: *** [/usr/local/sage/sage-6.2/local/var/lib/sage/installed/ipython-1.2.0] Error 1
make[2]: Leaving directory `/usr/local/sage/sage-6.2/build'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/usr/local/sage/sage-6.2/build'
 
real    24m14.305s

Obviously, I can get around this. However, I'm concerned about this entire approach to development...

comment:19 Changed 9 years ago by Volker Braun

You need to put the tarball manually in the upstream directory until the ticket is reviewed.

comment:20 in reply to:  19 Changed 9 years ago by William Stein

Replying to vbraun:

You need to put the tarball manually in the upstream directory until the ticket is reviewed.

That's what I did. It would be nice if the code that tries to grab the upstream from sagemath.org could also try other locations specified in the package, e.g., in this case https://pypi.python.org/packages/source/i/ipython/ipython-1.2.0.tar.gz...

I hope our longterm plan is that the process is:

   git <checkout a branch somehow>
   make

and it works, rather than

   git <checkout a branch somehow>
   make
   # see download failure -- track down packages -- put in upstream directory
   make
   # see more download failures -- track down packages -- put in upstream directory, etc.

?

comment:21 Changed 9 years ago by Volker Braun

The pypi url is not in the git branch. There are also some projects where you need to click through some html to get a temporary download link *cough* *sourceforge* *cough*. Really we need a common place where people can upload the tarballs.

comment:22 Changed 9 years ago by Jason Grout

+1 to moving to 1.2 now. I'm not running any extra patches beyond my 1.0 patches to get IPython master working. So if you merge this, I can stop maintaining the patches, even though I'm running master :).

And +1 to william's suggestion of having alternate URLs, since we are verifying the checksum anyway.

comment:23 Changed 9 years ago by Jason Grout

Note that IPython 1.2.1 is out, which fixes a bug they noticed just after 1.2

comment:24 Changed 9 years ago by William Stein

*IMPORTANT* This patch introduces this code in the middle of src/bin/sage-env *before* the path, etc., is properly set up:

if [ -x "$SAGE_LOCAL/bin/ipython" ]; then
    export IPYTHONDIR="$DOT_SAGE/ipython-"`ipython --version`
else
    # IPython is not yet installed
    export IPYTHONDIR="$DOT_SAGE/ipython-"`sed 's+\.p[0-9]*$++' "$SAGE_ROOT"/build/pkgs/ipython/package-version.txt`
fi

The result is that in a system with multiple version of Ipython (e.g., using python2.7.3 also) you'll get a big nasty ImportError: cannot import name MAXREPEAT error/traceback whenever you try to start sage (or use sage -sh). Moving the above IPYTHONDIR stuff to the bottom of the sage-env fixes the problem.

That said, it's kind of disturbing to be starting up ipython when doing "sage -sh", since that's actually a lot of overhead.

comment:25 in reply to:  23 Changed 9 years ago by William Stein

Replying to jason:

Note that IPython 1.2.1 is out, which fixes a bug they noticed just after 1.2

The upstream tarball is: https://pypi.python.org/packages/source/i/ipython/ipython-1.2.1.tar.gz

comment:26 Changed 9 years ago by William Stein

I know patches are stupid, but in case anybody cares, here's the patch to move the ipython version environ stuff and update the version of the tarball: http://wstein.org/tmp/ipython-1.2.1.patch

comment:27 Changed 9 years ago by git

Commit: 2ffb13cd82cc62d86e2a657ad0f5151a56a80e8c88a253c4a5d70855832d2de0ea258e9b0d0d9b7e

Branch pushed to git repo; I updated commit sha1. New commits:

3a0b07fipython: upgrade to version 1.2.1
061e7d6interpreter: preparser really is a stateless transformer
88a253cinterpreter: fix doctest issue

comment:28 Changed 9 years ago by git

Commit: 88a253c4a5d70855832d2de0ea258e9b0d0d9b7e0fb988d80d32913715f96bef242ff483f4c24b2f

Branch pushed to git repo; I updated commit sha1. New commits:

0fb988dsage-env: don't call ipython when creating the env

comment:29 Changed 9 years ago by R. Andrew Ohana

Description: modified (diff)
Status: newneeds_review
Summary: Update to IPython 1.2.0Update to IPython 1.2.1

comment:30 Changed 9 years ago by R. Andrew Ohana

Description: modified (diff)

comment:31 Changed 9 years ago by R. Andrew Ohana

Description: modified (diff)

comment:32 Changed 9 years ago by Jason Grout

You changed the preparser to a stateless transformer, but the preparse function *does* have state---it keeps track of the quote state, right?

comment:33 in reply to:  32 Changed 9 years ago by R. Andrew Ohana

Replying to jason:

You changed the preparser to a stateless transformer, but the preparse function *does* have state---it keeps track of the quote state, right?

Yes, although that seems like a hack that was only needed in old versions of ipython. Now ipython doesn't push an open ended quote state to the transformers, so the preparser doesn't actually need to keep track of this anymore.

I think there is a lot that can be done to clean up the preparser in general, but I think that is better fit for another ticket.

Last edited 9 years ago by R. Andrew Ohana (previous) (diff)

comment:34 Changed 9 years ago by William Stein

I've read all of this code pretty carefully, used it for months (in various incarnations by Jason and Andrew), and think upgrading to IPython 1.2.1 should be a very high priority for Sage. I'm running a clean build and round of doctests, and if they succeed, then this should get a positive review!

comment:35 Changed 9 years ago by Jason Grout

I'll do some checking too. My worry is not that the preparser could be cleaned up (it certainly could, but that's another issue), but that changing from a coroutine transformer to a stateless transformer is introducing bugs. I'm not sure why that change was made. Could you perhaps explain why it's not a coroutine transformer still?

comment:36 Changed 9 years ago by Jason Grout

oh, I see you did have an explanation. I'll look into it to make sure.

comment:37 Changed 9 years ago by William Stein

Status: needs_reviewpositive_review

Builds fine and all tests pass. The issues I pointed out above have also been addressed.

comment:38 Changed 9 years ago by Jason Grout

and I'm still building...

comment:39 Changed 9 years ago by Jason Grout

Giving a partial string as a command messes up the parser because the state is not reset, like "asdf. I'm figuring out how to exploit it.

comment:40 Changed 9 years ago by Jason Grout

All right +1. IPython *does* push open-ended quotes into the preparser if the line is a syntax error (like "a). However, the preparser is now always called with reset=True (the default), so the state doesn't carry over to the next line.

Here is a more complete explanation for it being okay to have a stateless transformation---it relies on two facts that are true right now:

  • we've switched the preparser to be an IPython python_line_transform (which assembles multiline strings into single lines)
  • the preparser has a default of reset=True, which means every time it is called, the state is discarded

If either of those changes, trouble would probably show up.

Other than that detail, this looks pretty much like the code I've been running on the sage cell server for nearly 6 months, so +1.

Last edited 9 years ago by Jason Grout (previous) (diff)

comment:41 Changed 9 years ago by Volker Braun

Reviewers: William Stein

comment:42 Changed 9 years ago by Volker Braun

Branch: u/ohanar/ipython-upgrade0fb988d80d32913715f96bef242ff483f4c24b2f
Resolution: fixed
Status: positive_reviewclosed

comment:43 Changed 9 years ago by Jeroen Demeyer

Commit: 0fb988d80d32913715f96bef242ff483f4c24b2f

Blocker follow-up at #15972

comment:44 Changed 8 years ago by Volker Braun

Random test failures due to history corruption at #16372

Note: See TracTickets for help on using tickets.