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

rcParams.keys() is not Python 3 compatible #1721

Merged
merged 1 commit into from Jan 30, 2013

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Jan 30, 2013

When I run rcParams.keys() in Python 3 I get the following exception:

AttributeError: 'dict_keys' object has no attribute 'sort'

This is due to the fact that the keys method does not return a list but a dict_keys object, which has no sort method.

To retain the behavior from Python 2 I suggest the following patch:

--- a/lib/matplotlib/__init__.py
+++ b/lib/matplotlib/__init__.py
@@ -733,7 +733,7 @@ See rcParams.keys() for a list of valid parameters.' % (key,))
         """
         Return sorted list of keys.
         """
-        k = dict.keys(self)
+        k = list(dict.keys(self))
         k.sort()
         return k

This works fine in Python 2 and 3.

For completeness, I used Python 3.2.3 and matplotlib 1.2.0 from Debian unstable/experimental.

Thanks for the great work.

@mdboom
Copy link
Member

mdboom commented Jan 30, 2013

The transformation you quote should already have been performed by 2to3. Are you perhaps importing matplotlib from the source tree, or from a 2.x install?

@tukss
Copy link
Author

tukss commented Jan 30, 2013

I agree that 2to3 should do this but it doesn't. You can test that easily by running 2to3 on __init__.py.
The reason is probably that we directly call the method of the dict class instead of the keys method of a dict instance. In the latter case 2to3 would add list() as expected.

So this seems to be a deficiency in 2to3 rather than matplotlib. I can report this to the Python bug tracker but I am not sure there will be a fix for that.

For the time being I guess we need a manual fix. What is your opinion?

@dmcdougall
Copy link
Member

There are no Travis failures indicating errors of this kind, so I am curious as to why you get this error. How does the error arise? Do you get it right after import matplotlib?

@dmcdougall
Copy link
Member

Addendum: AFAIK, Travis runs 2to3 on the whole source tree.

@tukss
Copy link
Author

tukss commented Jan 30, 2013

Generally, matplotlib in Python 3 works fine. I got this error while trying to modify rcParams from ipython (If you just type rcParams to see all parameters, you get the error). It's clearly a minor issue that can easily go unnoticed. I guess none of the tests try the keys() method of rcParams.

@dmcdougall
Copy link
Member

Has your ipython been built with python 3?

@tukss
Copy link
Author

tukss commented Jan 30, 2013

Yes, it has. This issue does not have anything to do with ipython, I just happened to notice it there first.

Steps to reproduce this with plain python3:

>>> import matplotlib
>>> matplotlib.rcParams.keys()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/matplotlib/__init__.py", line 726, in keys
    k.sort()
AttributeError: 'dict_keys' object has no attribute 'sort'

@mdboom
Copy link
Member

mdboom commented Jan 30, 2013

Interesting -- I see this now -- 2to3 is not covering this as I expected.

2to3 does do this:

 x = {}
-for k in x.keys():
+for k in list(x.keys()):
     pass

But leaves this alone (perhaps because there is an argument to keys here).

class Foo(dict):
    def keys(self):
        k = dict.keys(self)
        k.sort()
        return k

I'll go ahead and attach a commit with the change you recommended.

mdboom added a commit that referenced this pull request Jan 30, 2013
rcParams.keys() is not Python 3 compatible
@mdboom mdboom merged commit 02832c9 into matplotlib:v1.2.x Jan 30, 2013
@mdboom mdboom deleted the issue_1721 branch August 7, 2014 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants