Opened 7 years ago

Closed 6 years ago

#15990 closed enhancement (fixed)

Python 3 preparation: Change syntax of raise statement

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-6.2
Component: distribution Keywords: python3
Cc: tscrim Merged in:
Authors: Wilfried Luebbe Reviewers: R. Andrew Ohana
Report Upstream: N/A Work issues:
Branch: 136948a (Commits) Commit: 136948a88bbf731a916184b91adf52fb93daa0d4
Dependencies: Stopgaps:

Description

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.

Attachments (1)

failed-doctest-15990-SageDocTestRunner-line1206.txt (2.4 KB) - added by wluebbe 6 years ago.
Failed doc test in sage.doctest.forker.SageDocTestRunner?.report_unexpected_exception

Download all attachments as: .zip

Change History (27)

comment:1 Changed 6 years ago by wluebbe

  • Branch set to u/wluebbe/ticket/15990
  • Commit set to 9db8c5c598ec9de953a33b14e531bee3c092c199

comment:2 Changed 6 years ago by wluebbe

  • Status changed from new to needs_review

Changed 6 years ago by wluebbe

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

comment:3 Changed 6 years ago by wluebbe

  • Status changed from needs_review to needs_work

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

comment:4 Changed 6 years ago by git

  • Commit changed from 9db8c5c598ec9de953a33b14e531bee3c092c199 to 67ec20851353ba83e113a8c66d80561cbf2d0ef9

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

67ec208first run of futurize -w -f raise

comment:5 Changed 6 years ago by ohanar

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

comment:6 Changed 6 years ago by git

  • Commit changed from 67ec20851353ba83e113a8c66d80561cbf2d0ef9 to 3279bdc83784961786494580b003d3356b67a9d7

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

3279bdcfirst run of futurize -w -f raise

comment:7 Changed 6 years ago by wluebbe

Rebased on 6.2.beta6 and resolved merge conflicts.

comment:8 Changed 6 years ago by ohanar

  • Authors set to Wilfried Luebbe
  • Branch changed from u/wluebbe/ticket/15990 to u/ohanar/raise_statement
  • Commit changed from 3279bdc83784961786494580b003d3356b67a9d7 to f66c58281359d7535e8f488cfb96f096dd17ebd8
  • Reviewers set to R. Andrew Ohana
  • Status changed from needs_work to positive_review

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

comment:9 Changed 6 years ago by vbraun

  • Status changed from positive_review to needs_work
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

comment:10 Changed 6 years ago by git

  • Commit changed from f66c58281359d7535e8f488cfb96f096dd17ebd8 to 79b6367ee71ecd5137081076a86b841f53730e91

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

79b6367fix rebase error in 3279bdc

comment:11 Changed 6 years ago by ohanar

  • Status changed from needs_work to needs_review

comment:12 Changed 6 years ago by wluebbe

Hi Andrew, I am 20 minutes late with the same fix. Thanks! Fix resolves the above failure (tested OK!).

But my doctest ./sage -tp 4 --all --long >logs/sage-ptestlong-raise-0 had another failure:

File "src/sage/interfaces/expect.py", line 1017, in sage.interfaces.expect.Expect._expect_expr
Failed example:
    print sage0.eval("dummy=gp.eval('0'); alarm(1); gp._expect_expr('1')")  # long time
Expected:
    Control-C pressed.  Interrupting PARI/GP interpreter. Please wait a few seconds...
    ...
    AlarmInterrupt:
Got:
    Control-C pressed.  Interrupting PARI/GP interpreter. Please wait a few seconds...
    ---------------------------------------------------------------------------
    KeyboardInterrupt                         Traceback (most recent call last)
    <ipython-input-2-529203ce1422> in <module>()
    ----> 1 dummy=gp.eval('0'); alarm(Integer(Integer(1))); gp._expect_expr('1')
    <BLANKLINE>
    /home/wluebbe/sage-6.2.beta6/local/lib/python2.7/site-packages/sage/interfaces/expect.pyc in _expect_expr(self, expr, timeout)
       1050                 else:
       1051                     break
    -> 1052             raise KeyboardInterrupt(msg)
       1053 
       1054     def _sendstr(self, str):
    <BLANKLINE>
    KeyboardInterrupt:
**********************************************************************

The expectation AlarmInterrupt: was added by Jeroen in ticket:13311.

Am I the only one to see this failure?

comment:13 Changed 6 years ago by git

  • Commit changed from 79b6367ee71ecd5137081076a86b841f53730e91 to d4dc73495a917c3a00a947be1170aefc48fe3ea1

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

d4dc734fix raise statement autoconvert error

comment:14 Changed 6 years ago by wluebbe

Test passed for expect.py. Now starting ./sage -tp 4 --all --long >logs/sage-ptestlong-raise-4 to be sure.

comment:15 Changed 6 years ago by tscrim

  • Cc tscrim added

(Because I want to know when I might have potential conflicts.)

comment:16 Changed 6 years ago by wluebbe

  • Status changed from needs_review to positive_review

Finally: All tests passed! So it's positive.

comment:17 Changed 6 years ago by vbraun

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

comment:18 Changed 6 years ago by vbraun

  • Status changed from positive_review to needs_work

You can now merge in beta7

comment:19 Changed 6 years ago by wluebbe

  • Branch changed from u/ohanar/raise_statement to u/wluebbe/ticket/15990
  • Commit changed from d4dc73495a917c3a00a947be1170aefc48fe3ea1 to 16a0f9181ec2b636889018a4607e65920b5f2030
  • Status changed from needs_work to positive_review

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

comment:20 Changed 6 years ago by vbraun

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

comment:21 Changed 6 years ago by wluebbe

  • Branch changed from u/wluebbe/ticket/15990 to u/wluebbe/ticket/15990b
  • Commit 16a0f9181ec2b636889018a4607e65920b5f2030 deleted

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

Will it merge now :-/

comment:22 Changed 6 years ago by vbraun

Your branch doesn't exist...

comment:23 Changed 6 years ago by git

  • Commit set to 136948a88bbf731a916184b91adf52fb93daa0d4
  • Status changed from positive_review to needs_review

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

comment:24 Changed 6 years ago by wluebbe

Aaargh! I didn't push ...

comment:25 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

Merges cleanly.

Last edited 6 years ago by tscrim (previous) (diff)

comment:26 Changed 6 years ago by vbraun

  • Branch changed from u/wluebbe/ticket/15990b to 136948a88bbf731a916184b91adf52fb93daa0d4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.