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

Stop relying on 2to3 and use six.py for compatibility instead #2226

Merged
merged 3 commits into from Sep 3, 2013

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Aug 14, 2013

This is a significant amount of work, but will pay off (primarily in shorter build times) in the end.

@pelson
Copy link
Member

pelson commented Aug 12, 2013

primarily in shorter build times

And the fact that we would have a single code base which users of Python3 can actually contribute against...

@ghost ghost assigned mdboom Aug 13, 2013
@mdboom
Copy link
Member Author

mdboom commented Aug 13, 2013

Well, I'm not sure that using six vs. 2to3 requires any less knowledge of the differences between Python 2 and 3 to be effective at it, unfortunately. As I work through this, it certainly doesn't result in "cleaner" code.

@mdboom
Copy link
Member Author

mdboom commented Aug 14, 2013

Ok -- here it is. This is going to make some merges harder, unfortunately. Also, we should probably get #2281 (which is against 1.3.x) finished and merged with this before finalizing this. Some of these changes may be surprising, and I'll try to notate them here in this PR.

@@ -16,6 +16,10 @@
# For some later history, see
# http://thread.gmane.org/gmane.comp.python.matplotlib.devel/7068

from __future__ import absolute_import, division, print_function, unicode_literals

import six
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking here is to always include this in every file to make moving code around easier. The things it has are "core" enough that they should probably be considered "built-ins".

@efiring
Copy link
Member

efiring commented Aug 14, 2013

Mike,
Your explanations are very helpful. I think it would be a good idea to include them in the code as comments rather than leaving them only as comments in the PR. The things you are explaining are the sorts of things a person might trip over when working with the code later, so they are exactly the sort of thing for which in-code comments are needed. This will also be helpful to others, including myself, when trying to migrate other code from 2-only to single codebase 2-or-3.
Putting these examples and explanations in a file of some sort that becomes part of the documentation (not necessarily sphinxified--could be just a plain text file) would also be helpful.

@mdboom
Copy link
Member Author

mdboom commented Aug 14, 2013

For some of these, there are many instances of them, and I wouldn't want to comment them in each place. But perhaps a "writing portable code" document in the developer docs is in order.

@WeatherGod
Copy link
Member

And a reference to that document in the relevant source files for the most
tricky changes might also help bring that to the attention of any prying
eyes.

mdboom added a commit that referenced this pull request Sep 3, 2013
Stop relying on 2to3 and use `six.py` for compatibility instead
@mdboom mdboom merged commit 705fae3 into matplotlib:master Sep 3, 2013
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 24, 2013
…ters

too long according to pep8.  This bulk-fixes this by breaking
the import up into 4 seperate lines.
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Sep 25, 2013
…achters

too long according to pep8.  This bulk-fixes every file in the library.
@mdboom mdboom deleted the six branch August 7, 2014 13:54
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

4 participants