Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#15993 closed enhancement (fixed)

Python 3 preparation: Remove implicit tuple parameter unpacking

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-6.2
Component: distribution Keywords: python3
Cc: Merged in:
Authors: Wilfried Luebbe Reviewers: R. Andrew Ohana, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: fbbd41e (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by wluebbe)

The old syntax no more supported by Python 3.

Changes according to lib2to3/fixes/fix_tuple_params.py:

Fixer for function definitions with tuple parameters:
Replace
    def func(((a, b), c), d):
        ...
with
    def func(x, d):
        ((a, b), c) = x
        ...

It will also support lambdas:
    lambda (x, y): x + y -> lambda t: t[0] + t[1]
    # The parens are a syntax error in Python 3
    lambda (x): x + y -> lambda x: x + y

This ticket is tracked as a dependency of meta-ticket ticket:15980.

Change History (18)

comment:1 Changed 7 years ago by wluebbe

  • Description modified (diff)

comment:2 Changed 7 years ago by wluebbe

  • Branch set to u/wluebbe/ticket/15993
  • Commit set to cc7834ece9bbca2c363f8d4d07ac3e1675b8afff

Some patches still need to be updated manually (they contain strings like xxx_todo_changeme).


New commits:

cc7834echanges generated by 2to3 tool (tuple_params) for Python 3

comment:3 Changed 6 years ago by git

  • Commit changed from cc7834ece9bbca2c363f8d4d07ac3e1675b8afff to e178c9addc794706f97a81494a9bdf4877a0e277

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

e178c9afix manually the "xxx_todo_changeme" from previous commit

comment:4 Changed 6 years ago by wluebbe

  • Status changed from new to needs_review
./sage -t -p --all --long --logfile=logs/ptestlong-15993-c.log
...
All tests passed!

Note: .pyx modules were not adapted since 2to3 does not support cython modules.
But cython is said to be Python 3 compatible.

comment:5 Changed 6 years ago by chapoton

  • Status changed from needs_review to needs_work

needs to be rebased

comment:6 Changed 6 years ago by git

  • Commit changed from e178c9addc794706f97a81494a9bdf4877a0e277 to 347456e2b1c23909f3d18ed36f00f8943411f7c7

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

1e8d72cchanges generated by 2to3 tool (tuple_params) for Python 3
347456efix manually the "xxx_todo_changeme" from previous commit

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

  • Status changed from needs_work to needs_review

comment:9 Changed 6 years ago by chapoton

  • Branch changed from u/wluebbe/ticket/15993 to public/ticket/15993
  • Commit changed from 347456e2b1c23909f3d18ed36f00f8943411f7c7 to 45d499f62c0776fdf365fcd92f3eb071b23ed23d

I have made a few changes in a reviewer branch.


New commits:

45d499ftrac #15993 reviewer patch

comment:10 Changed 6 years ago by git

  • Commit changed from 45d499f62c0776fdf365fcd92f3eb071b23ed23d to fbbd41e8ad93e2660c23572460b7beaa3d20eb73

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

fbbd41etrac #15993 one minor error corrected

comment:11 Changed 6 years ago by chapoton

This looks good to me. I have made minor corrections ; it would be good that someone else checks again all the changes.

One should run the doctests, and then this will be good to go.

comment:12 Changed 6 years ago by ohanar

Looked them over, seem reasonable. I would be happy so long as the doctests pass.

comment:13 Changed 6 years ago by wluebbe

  • Authors set to Wilfried Luebbe

Looks good:

git fetch trac public/ticket/15993:public/ticket/15993
git co public/ticket/15993 
./sage -b
./sage -tp 4 --all --long >logs/sage-ptestlong-15993
#####
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 2392.2 seconds
    cpu time: 8143.0 seconds

comment:14 Changed 6 years ago by chapoton

  • Reviewers set to R. Andrew Ohana, Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, setting to positive

comment:15 Changed 6 years ago by vbraun

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

You can probably reproduce this if you merge in the latest beta...

comment:16 Changed 6 years ago by vbraun

Never mind, caused by #15990

comment:17 Changed 6 years ago by vbraun

  • Branch changed from public/ticket/15993 to fbbd41e8ad93e2660c23572460b7beaa3d20eb73
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:18 Changed 4 years ago by jdemeyer

  • Commit fbbd41e8ad93e2660c23572460b7beaa3d20eb73 deleted

See #20554 for a new version of this...

Note: See TracTickets for help on using tickets.