Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3 preparation: Change syntax of raise statement #15990

Closed
wluebbe mannequin opened this issue Mar 20, 2014 · 39 comments
Closed

Python 3 preparation: Change syntax of raise statement #15990

wluebbe mannequin opened this issue Mar 20, 2014 · 39 comments

Comments

@wluebbe
Copy link
Mannequin

wluebbe mannequin commented Mar 20, 2014

Only the modern syntax like raise E(V) and raise E(V).with_traceback(T) is accepted by Python 3.

Almost all cases change raise E, V to raise E(V) where V is a string.

TODO:

  • Clarify why the cases with_traceback were omitted in libfuturize/fixes/raise.py. (Perhaps this belongs to stage 2?)

Changes according to lib2to3/fixes/fix_raise.py:

raise         -> raise
raise E       -> raise E
raise E, V    -> raise E(V)
raise E, V, T -> raise E(V).with_traceback(T)
raise E, None, T -> raise E.with_traceback(T)

raise (((E, E'), E''), E'''), V -> raise E(V)
raise "foo", V, T               -> warns about string exceptions

CAVEATS:
   "raise E, V" will be incorrectly translated if V is an exception
   instance. The correct Python 3 idiom is
       raise E from V
   but since we can't detect instance-hood by syntax alone and since
   any client code would have to be changed as well, we don't automate
   this.

CC: @tscrim

Component: distribution

Author: Wilfried Luebbe

Branch/Commit: 136948a

Reviewer: R. Andrew Ohana

Issue created by migration from https://trac.sagemath.org/ticket/15990

@wluebbe wluebbe mannequin added this to the sage-6.2 milestone Mar 20, 2014
@wluebbe wluebbe mannequin added c: distribution labels Mar 20, 2014
@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 1, 2014

Branch: u/wluebbe/ticket/15990

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 1, 2014

Commit: 9db8c5c

@wluebbe wluebbe mannequin added the s: needs review label Apr 1, 2014
@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 1, 2014

Failed doc test in sage.doctest.forker.SageDocTestRunner.report_unexpected_exception

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 1, 2014

comment:3

Attachment: failed-doctest-15990-SageDocTestRunner-line1206.txt

In more than 500 py modules the raise statement was (simply) changed to the new syntax.

All doctest succeeded but one (see attachment):

./sage -t -p --all --logfile=logs/ptestlong-15990l-2.log
...
----------------------------------------------------------------------
sage -t src/sage/doctest/forker.py  # 1 doctest failed

I have no idea why ...

@wluebbe wluebbe mannequin added s: needs work and removed s: needs review labels Apr 1, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2014

Changed commit from 9db8c5c to 67ec208

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2014

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

67ec208first run of futurize -w -f raise

@ohanar
Copy link
Member

ohanar commented Apr 2, 2014

comment:5

If you can update the branch, I can maybe see if I can figure out what is going wrong in the forker.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2014

Changed commit from 67ec208 to 3279bdc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3279bdcfirst run of futurize -w -f raise

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 3, 2014

comment:7

Rebased on 6.2.beta6 and resolved merge conflicts.

@ohanar
Copy link
Member

ohanar commented Apr 3, 2014

comment:8

It looks like it is just a bug in the doctest (it is testing the debugger, so the actual source code ends up being in the output to check).

Otherwise, looks good. (Hopefully this will get merged ASAP before it there are a ton of conflicts again).


New commits:

f66c582fix doctest for trac #15990

@ohanar
Copy link
Member

ohanar commented Apr 3, 2014

Reviewer: R. Andrew Ohana

@ohanar
Copy link
Member

ohanar commented Apr 3, 2014

Author: Wilfried Luebbe

@ohanar
Copy link
Member

ohanar commented Apr 3, 2014

Changed branch from u/wluebbe/ticket/15990 to u/ohanar/raise_statement

@ohanar
Copy link
Member

ohanar commented Apr 3, 2014

Changed commit from 3279bdc to f66c582

@vbraun
Copy link
Member

vbraun commented Apr 4, 2014

comment:9
File "src/sage/combinat/designs/ext_rep.py", line 737, in sage.combinat.designs.ext_rep.XTree.__getitem__
Failed example:
    xt.__getitem__(119)
Expected:
    Traceback (most recent call last):
    ...
    IndexError: XTree<blocks> has no index 119
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.combinat.designs.ext_rep.XTree.__getitem__[4]>", line 1, in <module>
        xt.__getitem__(Integer(119))
      File "/home/release/Sage/local/lib/python2.7/site-packages/sage/combinat/designs/ext_rep.py", line 745, in __getitem__
        raise IndexError('%s no index %s' % (self.__repr__(), `i`))
    IndexError: XTree<blocks> no index 119

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 4, 2014

Changed commit from f66c582 to 79b6367

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 4, 2014

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

79b6367fix rebase error in 3279bdc

@vbraun
Copy link
Member

vbraun commented Apr 4, 2014

comment:17

Merge conflicts, you'll probably have to wait for the next beta and merge that in.

@vbraun
Copy link
Member

vbraun commented Apr 8, 2014

comment:18

You can now merge in beta7

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

comment:19

After resolving merge conflicts still tests OK

All tests passed!
----------------------------------------------------------------------
Total time for all tests: 2417.5 seconds
    cpu time: 8135.6 seconds

New commits:

16a0f91Merge branch 'develop' into u/wluebbe/ticket/15990

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

Changed branch from u/ohanar/raise_statement to u/wluebbe/ticket/15990

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

Changed commit from d4dc734 to 16a0f91

@wluebbe wluebbe mannequin added s: positive review and removed s: needs work labels Apr 9, 2014
@vbraun
Copy link
Member

vbraun commented Apr 9, 2014

comment:20

Conflicts again with #15148 and #11726 which I already merged after beta7

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

Changed commit from 16a0f91 to none

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

Changed branch from u/wluebbe/ticket/15990 to u/wluebbe/ticket/15990b

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

comment:21

I merged with ticket:15148 and ticket:11726 and tested.

Will it merge now :-/

@vbraun
Copy link
Member

vbraun commented Apr 9, 2014

comment:22

Your branch doesn't exist...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2014

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

e07c76eMerge branch 'public/ticket/11726' of trac.sagemath.org:sage into 11726
eb09160trac #11726 implement floordiv for laurent polys in one variable
e27a420trac #11726 one failing doctest corrected
ba866efMerge branch 'public/ticket/11726' into raise-plus
86261b1Improve count_points
55cdcf3Refactor point counting code for hyperelliptic curves.
fc7a569First bunch of fixes and missing examples.
63bf717Rebase on top of #15108.
8029bc6Merge remote-tracking branch 'trac/develop' into ticket/15148
136948aMerge branch 'u/jpflori/ticket/15148' into raise-plus

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 9, 2014

Commit: 136948a

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Apr 9, 2014

comment:24

Aaargh! I didn't push ...

@tscrim
Copy link
Collaborator

tscrim commented Apr 9, 2014

comment:25

Merges cleanly.

@vbraun
Copy link
Member

vbraun commented Apr 9, 2014

Changed branch from u/wluebbe/ticket/15990b to 136948a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants