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 iteritems #33726

Closed
fchapoton opened this issue Apr 18, 2022 · 15 comments
Closed

get rid of some iteritems #33726

fchapoton opened this issue Apr 18, 2022 · 15 comments

Comments

@fchapoton
Copy link
Contributor

Change iteritems to items in two py files where iteritems makes no sense with python3.

Also in one pyx file. Note that in pyx files, with python3, items is the same as iteritems.

CC: @tscrim

Component: refactoring

Author: Frédéric Chapoton

Branch: 0d580e5

Reviewer: Kwankyu Lee

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

@fchapoton fchapoton added this to the sage-9.6 milestone Apr 18, 2022
@fchapoton
Copy link
Contributor Author

Commit: aa51084

@fchapoton
Copy link
Contributor Author

New commits:

aa51084get rid of some iteritems

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/33726

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 19, 2022

comment:2

in two py files where this makes no sense

What do you mean? You only replaced iteritems with items (of python3).

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 19, 2022

comment:3

While we are at it, why not change for t,g in ... to for t, g in ...?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 19, 2022

Changed commit from aa51084 to 0d580e5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 19, 2022

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

0d580e5more spaces after comma

@fchapoton
Copy link
Contributor Author

comment:5

Replying to @kwankyu:

in two py files where this makes no sense

What do you mean? You only replaced iteritems with items (of python3).

Well, I suspect that this part of the code is never doctested. In python3, dictionaries no longer have an iteritems method.

The only thing that still have an iteritems method are sage own's weak dictionaries. We should switch this method's name to items, but not in this ticket.

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 19, 2022

Reviewer: Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 19, 2022

comment:6

Replying to @fchapoton:

Replying to @kwankyu:

in two py files where this makes no sense

What do you mean? You only replaced iteritems with items (of python3).

Well, I suspect that this part of the code is never doctested. In python3, dictionaries no longer have an iteritems method.

Right.

The only thing that still have an iteritems method are sage own's weak dictionaries. We should switch this method's name to items, but not in this ticket.

Okay.

@kwankyu

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented May 24, 2022

Changed branch from u/chapoton/33726 to 0d580e5

@slel
Copy link
Member

slel commented May 31, 2022

Changed commit from 0d580e5 to none

@slel
Copy link
Member

slel commented May 31, 2022

comment:8

This change missed one of the iteritems:

-        for t,g in self._t_dict.iteritems():
-            for k,c in g.monomial_coefficients(copy=False).iteritems():
+        for t, g in self._t_dict.items():
+            for k, c in g.monomial_coefficients(copy=False).iteritems():

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 3, 2022

comment:9

Replying to Frédéric Chapoton:

The only thing that still have an iteritems method are sage own's weak dictionaries. We should switch this method's name to items, but not in this ticket.

I've opened #34488 for this

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

5 participants