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: The semantic of filter() function is changed #16067

Closed
wluebbe mannequin opened this issue Apr 7, 2014 · 27 comments
Closed

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

wluebbe mannequin opened this issue Apr 7, 2014 · 27 comments

Comments

@wluebbe
Copy link
Mannequin

wluebbe mannequin commented Apr 7, 2014

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.

CC: @a-andre

Component: distribution

Author: Wilfried Luebbe

Branch/Commit: 49e52c3

Reviewer: André Apitzsch

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

@wluebbe wluebbe mannequin added this to the sage-6.2 milestone Apr 7, 2014
@wluebbe wluebbe mannequin added c: distribution labels Apr 7, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 20, 2014

Commit: e574c68

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 20, 2014

comment:2

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"

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 20, 2014

Branch: u/wluebbe/ticket/16067

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 20, 2014

comment:3

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

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 21, 2014

comment:4

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!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2014

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2014

Changed commit from e574c68 to 65c8620

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 21, 2014

comment:6

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!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2014

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

1d50592trac #16067: tidying up the generated patch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 21, 2014

Changed commit from 65c8620 to 1d50592

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 21, 2014

Author: Wilfried Luebbe

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 21, 2014

comment:8

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

This ticket may really belong to stage 1 ...

@wluebbe wluebbe mannequin added the s: needs review label May 21, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2014

Changed commit from 1d50592 to c68a033

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 26, 2014

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

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

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented May 29, 2014

comment:10

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

@a-andre
Copy link
Contributor

a-andre commented May 31, 2014

comment:11

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)]

@a-andre
Copy link
Contributor

a-andre commented May 31, 2014

Reviewer: André Apitzsch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 4, 2014

Changed commit from c68a033 to 0cebea1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 4, 2014

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 5, 2014

Changed commit from 0cebea1 to 0c2486e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 5, 2014

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

0c2486etrac #16067: answered reviewer comments and further improvements

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Jun 5, 2014

comment:14

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

@wluebbe wluebbe mannequin added s: needs review and removed s: needs work labels Jun 5, 2014
@a-andre
Copy link
Contributor

a-andre commented Jun 5, 2014

comment:15

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2014

Changed commit from 0c2486e to 49e52c3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 6, 2014

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

49e52c3trac #16067: fixed duplicate lines found by reviewer

@wluebbe
Copy link
Mannequin Author

wluebbe mannequin commented Jun 6, 2014

comment:17

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

"Eile mit Weile ..."

@vbraun
Copy link
Member

vbraun commented Jun 6, 2014

Changed branch from u/wluebbe/ticket/16067 to 49e52c3

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

2 participants