dict.items() not converted to list(dict.items()) when iterated over
hniksic opened this issue · 4 comments
It appears that python-modernize
doesn't convert for k, v in d.items()
to for k, v in list(d.items())
, assuming that the use of items()
was an accident in Python 2. This is not always the case - consider the following two functions:
def foo1(d):
return d.items()
def foo2(d):
for k, v in d.items():
if v % 2 == 0:
del d[k]
When python-modernize
is run on the code, it correctly modifies foo1
to return list(d.items())
, but it doesn't modify foo2
to iterate over list(d.items())
. In our code base this led to many subtle bugs because our Python 2 code was very careful to only use items()
(and keys()
and values()
) because the dictionary was being modified.
Is there a way to opt out of the special-handling of items
inside for
?
There isn't a command-line option or anything, but it looks like modernize is modifying the handling from 2to3 slightly to assume for k, v in d.items()
is a mistake, rather than deliberate. So you could just comment out the modification and use the logic from 2to3:
@takluyver Thanks for the help. I saw this part of the source, but my question (and this issue) is about enabling this without modifying the source, for the sake of others who encounter this issue.
I would also argue that the default is incorrect and should be changed because it breaks correct code, as shown in the issue description. But this might be a matter of opinion, and explicit opt-out would certainly work for us.
modernize generally doesn't go in for a lot of options - what you can do is mostly enable and disable specific fixers. You could always disable this one and run the one from 2to3, or one you've customised.
Thinking about it, there was a push a while back to make modernize idempotent, so running it on code you had already modernize-d wouldn't produce further changes. I guess this override was part of that, because without it for k in d.iterkeys()
-> for k in d.keys()
-> for k in list(d.keys())
.
@takluyver The idempotence argument would also require return d.keys()
not to be transformed to return list(d.keys())
. I am aware that I could work around the problem by not using the fixer in question and switching t 2to3
for that particular fix, but I would prefer an improvement python-modernize
correctly, as we otherwise find it a huge help with migration to Python 3.