Opened 8 years ago

Closed 8 years ago

#16067 closed enhancement (fixed)

Python 3 preparation: The semantic of filter() function is changed

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-6.3
Component: distribution Keywords: python3
Cc: aapitzsch Merged in:
Authors: Wilfried Luebbe Reviewers: André Apitzsch
Report Upstream: N/A Work issues:
Branch: 49e52c3 (Commits, GitHub, GitLab) Commit: 49e52c3a05223943a6f2777cb336ca68c294fc73
Dependencies: Stopgaps:

Status badges

Description

In Py2 filter() returns a list, while in Py3 filter() returns an iterator (as itertools.ifilter() does in Py2).

The tool 2to3 wraps filter() usages with a call to list().
An alternative approach is to add from future_builtins import filter and to check where a wrapping with list() is required.

There are 53 effected modules.

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

Change History (19)

comment:1 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:2 Changed 8 years ago by wluebbe

  • Branch set to u/wluebbe/ticket/16067
  • Commit set to e574c6856980b83c05e687c4f8a50119972789c0

2to3 does even better as its documentation ​2to3 filter suggests:
"Wraps filter() usage in a list call."

Often it changes the code as indicated by ​filter():
"Note that filter(function, iterable) is equivalent to [item for item in iterable if function(item)] if function is not None and [item for item in iterable if item] if function is None."

This is more readable and more pythonic as a combination of filter and lamda. And it allows sometimes further improvements!

New commits:

​e574c68trac #16067: changes generated by "2to3 -w -f filter src/sage"

comment:3 Changed 8 years ago by wluebbe

These are the generated changes. Some manual fixups / improvements to come.

comment:4 Changed 8 years ago by wluebbe

Apparently I did not run ./sage -b before the test - Grrr ... :-(

Now I will always use ./sage -b;./sage -tp 4 --all --long >logs/sage-tp4-all-long-16067.log in one line!

comment:5 Changed 8 years ago by git

  • Commit changed from e574c6856980b83c05e687c4f8a50119972789c0 to 65c86200df14be22f4af62c494df1ad702ce7e14

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

​65c8620trac #16067: fix an error generated by "2to3"

comment:6 Changed 8 years ago by wluebbe

In sage/graphs/generic_graph.py the name filter was overwritten (by a local function definition. Therefore 2to3 got confused and wrapped the call filter() with list(). This caused a lot of doctest failures and 4 segfaults!

comment:7 Changed 8 years ago by git

  • Commit changed from 65c86200df14be22f4af62c494df1ad702ce7e14 to 1d505928a109bdb8cfb358fa6c8ef8df257981d9

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

​1d50592trac #16067: tidying up the generated patch

comment:8 Changed 8 years ago by wluebbe

  • Authors set to Wilfried Luebbe
  • Status changed from new to needs_review

I think this a complete fix for the filter() function.

This ticket may really belong to stage 1 ...

comment:9 Changed 8 years ago by git

  • Commit changed from 1d505928a109bdb8cfb358fa6c8ef8df257981d9 to c68a033ee1b8a8dcd886a55926fb593ef60662ba

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

​c68a033Merge branch 'develop' into u/wluebbe/ticket/16067

comment:10 Changed 8 years ago by wluebbe

  • Cc aapitzsch added

Hi André, would you like to review this Py3 ticket?

comment:11 Changed 8 years ago by aapitzsch

  • Reviewers set to André Apitzsch
  • Status changed from needs_review to needs_work

Hi, please change ==None back to is None in src/sage/combinat/k_tableau.py.

Could you check that the indentions are correct, e.g. in src/sage/misc/sagedoc.py: 

match_list = [s for s in match_list if re.search(extra, s[1],
                                  re.MULTILINE | flags)]

should be 

match_list = [s for s in match_list if re.search(extra, s[1],
                                                 re.MULTILINE | flags)]

comment:12 Changed 8 years ago by git

  • Commit changed from c68a033ee1b8a8dcd886a55926fb593ef60662ba to 0cebea10fe118ba5bfb322bc9a559fe424bde238

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

​0cebea1Merge branch 'develop' into u/wluebbe/ticket/16067

comment:13 Changed 8 years ago by git

  • Commit changed from 0cebea10fe118ba5bfb322bc9a559fe424bde238 to 0c2486edf69cec395da0af127570fef6e56074a7

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

​0c2486etrac #16067: answered reviewer comments and further improvements

comment:14 Changed 8 years ago by wluebbe

  • Status changed from needs_work to needs_review

Resolved merge conflict, answered the reviewer comment and added a few additional improvements. All doctests pass.

comment:15 Changed 8 years ago by aapitzsch

  • Status changed from needs_review to needs_work

Check your changes in src/sage/doctest/control.py and src/sage/quadratic_forms/quadratic_form__ternary_Tornaria.py. Some functionality appears twice.

comment:16 Changed 8 years ago by git

  • Commit changed from 0c2486edf69cec395da0af127570fef6e56074a7 to 49e52c3a05223943a6f2777cb336ca68c294fc73

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

​49e52c3trac #16067: fixed duplicate lines found by reviewer

comment:17 Changed 8 years ago by wluebbe

  • Status changed from needs_work to needs_review

Hi André, thank you for reviewing and finding these problems! Apparently I wanted to go too fast :-/

"Eile mit Weile ..."

comment:18 Changed 8 years ago by aapitzsch

  • Status changed from needs_review to positive_review

comment:19 Changed 8 years ago by vbraun

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