fix_dict.py applies fixes to every instance of keys(), items() or values() irrespective of the type of object. Since 2to3 cannot check the type of the object, we can at least add the check to the generated code like...

d.keys() -> list(d.keys) if type(d) == dict else d.keys()
and similarly 

d.viewkeys() -> d.keys() if type(d) == dict else d.viewkeys()

PFA the tweaked fixer.

Thanks for the patch but I feel adding type checks will add more complexity to the code being converted and there are a lot of cases to handle. I have some code below where running 2to3 will just wrap the keys() call to a list but with the patch the if call might be have an inner if call if I am understanding the patch correctly as you have handled the for loop case.


$ cat ../backups/bpo34978.py
a = {1: 1}

True if 1 in a.keys() else False
$ 2to3 -w ../backups/bpo34978.py
RefactoringTool: Skipping optional fixer: buffer
RefactoringTool: Skipping optional fixer: idioms
RefactoringTool: Skipping optional fixer: set_literal
RefactoringTool: Skipping optional fixer: ws_comma
RefactoringTool: Refactored ../backups/bpo34978.py
--- ../backups/bpo34978.py	(original)
+++ ../backups/bpo34978.py	(refactored)
@@ -1,3 +1,3 @@
 a = {1: 1}

-True if 1 in a.keys() else False
+True if 1 in list(a.keys()) else False
RefactoringTool: Files that need to be modified:
RefactoringTool: ../backups/bpo34978.py
➜  cpython git:(master) cat ../backups/bpo34978.py
a = {1: 1}

True if 1 in a.keys() else False


With the patch the above might be as below which throws syntax error.

True if 1 in list(a.keys()) if type(a) == dict else a.keys() else False

I think this is one case which can be handled like for but there might be other places where this might return invalid results that I couldn't think of like nested if clauses in list comprehensions and so on. I think this fixer was written with the notion that .keys() and .values() is a very common method for dict and these methods are not something many people define themselves as far as I have seen from other's code since they are more attached with dict so that the fixer affects those cases. I think the gain is minimal here with cases to handle. 

`list(d.keys())` seems to be more Pythonic than `list(d.keys) if type(d) == dict else d.keys()` though it provides some additional type checks.

# With current 2to3

keys = list(a.keys())

if 1 in list(a.keys()):
    True
else:
    False



# With patch this also increases line length and more to understand when looking at the code

keys = list(a.keys()) if type(a) == dict else a.keys()

if 1 in list(a.keys()) if type(a) == dict else a.keys():
    True
else:
    False


I couldn't find any discussions at the moment to see if this was discussed earlier when 2to3 was written. This is just my suggestion and I will leave it to Benjamin and others for thoughts on this. Feel free to correct me if I am misunderstanding the patch where it was handled.


Thanks

Thank you very much for pointing out some helpful things,  Karthikeyan. 

>True if 1 in list(a.keys()) if type(a) == dict else a.keys() else False

True that the fixer would return a syntax error in this case. I missed adding a pair of parenthesis to the whole if statement. I mean

True if 1 in (list(a.keys()) if type(a) == dict else  a.keys()) else False

is valid code. So adding parenthesis would solve the problem as I did in the updated patch I uploaded.

I don't see this updated fixer returning errors in other cases like nested if conditions or list comprehensions, since the fixer would convert the respective function calls of dict objects into separate statements, would not interfere with other if statements(or any other statements for that matter) and set the order of precedence appropriately.

Please do point out if there any other cases I missed. 

I know this is a less pythonic but is more accurate :) Thanks.

I'm disinclined to change the fix here after it's existed in this for for 10 years.