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

Support Python 3 #233

Merged
merged 7 commits into from Feb 23, 2014
Merged

Support Python 3 #233

merged 7 commits into from Feb 23, 2014

Conversation

ghost
Copy link

@ghost ghost commented Feb 22, 2014

Part of #232

Cola now runs on both Python 2.6+ and 3! Many refactorings were
necessary for this. The first commits are about modernizing the py2 codebase and adding necessary __future__ imports, and the last commit is the real thing.

In-house "six-like" compat unit: Because there were only a few
compatibility tricks needed, I felt that it was overkill to add a
dependency on the "six" module, so I added the necessary compatibility
tricks in cola.compat:

  • ustr: replaces all previous instances of unicode.
  • unichr: Alias of chr under py3.
  • StringIO: Import from cStringIO under py2 and io under py3.
  • urllib: Aliases to urllib.parse under py3.

Then, there's a couple of small fixes that had to be made:

  • StandardError --> Exception
  • except foo, e --> except foo as e
  • foo.reverse() --> reversed(foo)
  • xrange() --> range() (minimal perf impact under py2)
  • iteritems() --> items() (minimal perf impact under py2)
  • Wrapping list() where needed (filter(), map(), etc.)
  • __next__ = next aliasing

Also, there were a couple of places where core.encode() was used and
didn't have to. Under py2, it did nothing, but under py3, it generated
an error. I've tried to be as careful and minimal as possible for that
part, but if there's one area where there's a risk of newly indtroduced
bugs, it's that part.

There were also "cmp" functions that had to be replaced with "key"
functions.

Virgil Dupras added 6 commits February 21, 2014 16:58
Added "from __future__ import division" in every python unit of the
project. Wherever there was division going on, ensured that it was still
correct and modern.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
That allowed us to remove some dead weight in cola.compat.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
No further adjustments were required, the code was already clear of
ambiguous imports.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
ProgressIndicator, which isn't used anywhere, is the only place where we
use a print statement. Rather than modernizing it with a
"print_function" __future__, I've simply removed it.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
Also, replaced the few "u" literals spread across the code with normal
string literals.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
Cola now runs on both Python 2.6+ and 3! Many refactorings were
necessary for this.

In-house "six-like" compat unit: Because there were only a few
compatibility tricks needed, I felt that it was overkill to add a
dependency on the "six" module, so I added the necessary compatibility
tricks in cola.compat:

* ustr: replaces all previous instances of `unicode`.
* unichr: Alias of `chr` under py3.
* StringIO: Import from `cStringIO` under py2 and `io` under py3.
* urllib: Aliases to `urllib.parse` under py3.

Then, there's a couple of small fixes that had to be made:

* StandardError --> Exception
* except foo, e --> except foo as e
* foo.reverse() --> reversed(foo)
* xrange() --> range() (minimal perf impact under py2)
* iteritems() --> items() (minimal perf impact under py2)
* Wrapping list() where needed (filter(), map(), etc.)
* __next__ = next aliasing

Also, there were a couple of places where `core.encode()` was used and
didn't have to. Under py2, it did nothing, but under py3, it generated
an error. I've tried to be as careful and minimal as possible for that
part, but if there's one area where there's a risk of newly indtroduced
bugs, it's that part.

There were also "cmp" functions that had to be replaced with "key"
functions.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
@davvid
Copy link
Member

davvid commented Feb 23, 2014

Looks great!I think once we fixup these two small issues it'll be good to go. Yeah, StringIO instead of cStringIO would be a fine solution. It definitely has saner behavior.

>>> from StringIO import StringIO
>>> io = StringIO()
>>> io.write(unichr(0x1234))
>>> io.getvalue()
u'\u1234'

It turns out that Python has its `io` modules since Python 2.6,
something I didn't know. It simplifies the two previous `cStringIO` uses
and removes the need for a `compat.StringIO`.

Signed-off-by: Virgil Dupras <hsoft@hardcoded.net>
@ghost
Copy link
Author

ghost commented Feb 23, 2014

Well, it turns out that Python added its io module in v2.6. I didn't know that. That means that we don't need a compat.StringIO and we can simply use io.StringIO everywhere.

I've tested the new code with non-ascii data under both v2.7 and v3.3 and it worked fine.

davvid added a commit that referenced this pull request Feb 23, 2014
Support Python 3

Signed-off-by: David Aguilar <davvid@gmail.com>
@davvid davvid merged commit 539a0c4 into git-cola:master Feb 23, 2014
@davvid
Copy link
Member

davvid commented Feb 23, 2014

Awesome, thank you!

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

1 participant