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 six.iterkeys #29077

Closed
fchapoton opened this issue Jan 25, 2020 · 11 comments
Closed

get rid of six.iterkeys #29077

fchapoton opened this issue Jan 25, 2020 · 11 comments

Comments

@fchapoton
Copy link
Contributor

only 3 files are touched

CC: @tscrim @jm58660

Component: python3

Author: Frédéric Chapoton

Branch/Commit: 8d9ecb1

Reviewer: Travis Scrimshaw

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

@fchapoton fchapoton added this to the sage-9.1 milestone Jan 25, 2020
@fchapoton
Copy link
Contributor Author

New commits:

238b198get rid of all calls to six.iterkeys

@fchapoton
Copy link
Contributor Author

Commit: 238b198

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/29077

@fchapoton
Copy link
Contributor Author

comment:2

green bot, please review

@tscrim
Copy link
Collaborator

tscrim commented Jan 26, 2020

comment:4

This is kind of a while-we-are-at-it thing, but I think we can avoid the double call to f.dict() and g.dict() here:

         if not coeff:
             coeff = self.base_ring().one()
         else:
-            coeff = self.base_ring()(next(itervalues(f.dict())) /  next(itervalues(g.dict())))
+            nf = next(iter(f.dict().values()))
+            ng = next(iter(g.dict().values()))                    # (trailing whitespace too)
+            coeff = self.base_ring()(nf / ng)
 
-        f = next(iterkeys(f.dict()))
-        g = next(iterkeys(g.dict()))
+        f = next(iter(f.dict()))
+        g = next(iter(g.dict()))

by instead doing

        fd = f.dict()
        gd = g.dict()
        fk = next(iter(fd))
        gk = next(iter(gd))

        if not coeff:
            coeff = self.base_ring().one()
        else:
            coeff = self.base_ring()(fd[fk] / gd[gk])

        f = fk
        g = gk

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 26, 2020

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

8d9ecb1trac 29077 suggested improvement

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 26, 2020

Changed commit from 238b198 to 8d9ecb1

@fchapoton
Copy link
Contributor Author

comment:6

Thanks for the review. Here is a proposal, more or less what you suggested.

@tscrim
Copy link
Collaborator

tscrim commented Jan 26, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jan 26, 2020

comment:7

That is probably a better idea. Thanks.

@vbraun
Copy link
Member

vbraun commented Jan 31, 2020

Changed branch from u/chapoton/29077 to 8d9ecb1

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