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

get rid of some more useless .keys in combinat #21379

Closed
fchapoton opened this issue Aug 31, 2016 · 14 comments
Closed

get rid of some more useless .keys in combinat #21379

fchapoton opened this issue Aug 31, 2016 · 14 comments

Comments

@fchapoton
Copy link
Contributor

sequel of #21296 and #21304

with an eye towards python3 compatibility

when iterating over a dict, one does not need to call .keys()

and when asking for the list of keys, it is better to call list(d) for compatibility with py3

Removing .keys() when possible is performed in some files in combinat folder

Component: combinatorics

Author: Frédéric Chapoton

Branch/Commit: 8021e24

Reviewer: Travis Scrimshaw

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

@fchapoton fchapoton added this to the sage-7.4 milestone Aug 31, 2016
@fchapoton
Copy link
Contributor Author

New commits:

c92288bmore .keys() removal inside combinat

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/21379

@fchapoton
Copy link
Contributor Author

Commit: c92288b

@tscrim
Copy link
Collaborator

tscrim commented Aug 31, 2016

comment:2

Two stupid things. You made a PEP8 change which made me notice that while-we-are-at-it^(TM), could you make this change:

-        if len(positions) == 0:
+        if not positions:

Also, in CyclicSievingPolynomial, you can replace keys by orbit_sizes.

@tscrim
Copy link
Collaborator

tscrim commented Aug 31, 2016

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2016

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

f9ab401trac 21379 some details after review

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 31, 2016

Changed commit from c92288b to f9ab401

@fchapoton
Copy link
Contributor Author

comment:5

done. And thank you again for taking time to do so many reviews.

@tscrim
Copy link
Collaborator

tscrim commented Aug 31, 2016

comment:6

It's nothing compared to what you've been doing.

@vbraun
Copy link
Member

vbraun commented Sep 3, 2016

comment:7

See patchbot

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 3, 2016

Changed commit from f9ab401 to 8021e24

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 3, 2016

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

cae55cfMerge branch 'u/chapoton/21379' in 7.4.b3
8021e24trac 21379 correct cyclic sieving

@fchapoton
Copy link
Contributor Author

comment:9

ok, I have corrected the problem.

@vbraun
Copy link
Member

vbraun commented Sep 4, 2016

Changed branch from u/chapoton/21379 to 8021e24

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

3 participants