Opened 7 years ago

Closed 6 years ago

#15984 closed enhancement (fixed)

Python 3 preparation: Change some code to use more modern Python idioms

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-6.2
Component: distribution Keywords: python3
Cc: Merged in:
Authors: Wilfried Luebbe Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 6964533 (Commits) Commit: 6964533eb27d07a2d1bcccb0dbfc7214edb57b16
Dependencies: Stopgaps:

Description

This fixer changes code like type(x) == T) and while 1: and it introduces the sorted() function where appropriate.

These changes are optional, but they improve style and may provide a small performance gain.

Changes according to lib2to3/fixes/fix_idioms.py:

* Change some type comparisons to isinstance() calls:
    type(x) == T -> isinstance(x, T)
    type(x) is T -> isinstance(x, T)
    type(x) != T -> not isinstance(x, T)
    type(x) is not T -> not isinstance(x, T)

* Change "while 1:" into "while True:".

* Change both
    v = list(EXPR)
    v.sort()
    foo(v)

and the more general
    v = EXPR
    v.sort()
    foo(v)

into
    v = sorted(EXPR)
    foo(v)

Change History (9)

comment:1 Changed 7 years ago by wluebbe

  • Branch set to u/wluebbe/ticket/15984
  • Commit set to 85f7d5e4d37663aa841e33a52664d1e070c32fa6
  • Status changed from new to needs_review
  • At first I applied the 2to3 tool (idioms) -> first commit
  • Fixing the doctests failures of the first commit -> second commit. All tests passed.
  • Changed while 1: into while True: for cython modules -> third commit.

New commits:

86de429changes made by 2to3 tool (idioms), many changes
1f05981reverted changes to 4 modules done by 2to3 tool
85f7d5echange "while 1:" into "while True:" for .pyx modules

comment:2 Changed 6 years ago by git

  • Commit changed from 85f7d5e4d37663aa841e33a52664d1e070c32fa6 to 6964533eb27d07a2d1bcccb0dbfc7214edb57b16

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

617b158changes made by 2to3 tool (idioms), many changes
85ef438reverted changes to 4 modules done by 2to3 tool
6964533change "while 1:" into "while True:" for .pyx modules

comment:3 Changed 6 years ago by wluebbe

Rebased on 6.2.beta6 and resolved merge conflicts.

comment:4 Changed 6 years ago by wluebbe

And the test report:

All tests passed!
----------------------------------------------------------------------
Total time for all tests: 3214.7 seconds

comment:5 Changed 6 years ago by tscrim

I think it would also be good to change x == None to x is None while we're at it.

comment:6 Changed 6 years ago by wluebbe

There are 151 modules (py and pyx combined) which have at least one of == None or != None.
The change makes sense as (by far) most modules use is None or is not None.

But as this change is already pretty large I would prefer to see this change in a separate ticket/branch.

comment:7 Changed 6 years ago by tscrim

I think it fits well with this ticket (plus how many of those modules overlap with this one?). Also one large sweeping change is the same to me as two large sweeping changes (especially with different commits), but with the added benefit of an only one time potential rebase. If you still disagree and want it in a separate ticket, then you can set this to positive review.

comment:8 Changed 6 years ago by wluebbe

  • Authors set to Wilfried Luebbe
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

I generated 151 patched py and pyx files. 3 pyx files gave doctest failures. And I got merge conflicts with this branch.

Therefor I set this ticket to "positive review" (as suggested above).

I will open a separate ticket for == None and generate fresh patches when this ticket is merged in a beta or release.

comment:9 Changed 6 years ago by vbraun

  • Branch changed from u/wluebbe/ticket/15984 to 6964533eb27d07a2d1bcccb0dbfc7214edb57b16
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.