darvid / hydrogen

Lightweight pip wrapper with bower support and an opinionated, npm-like workflow.
http://darvid.github.io/projects/hydrogen/
BSD 2-Clause "Simplified" License
3 stars 1 forks source link

Python3 compatibility fixes #1

Closed bbenne10 closed 8 years ago

bbenne10 commented 8 years ago

It looks like this was written with python3 in mind, but only tested against python 2. This PR should fix the python3 implementation while not affecting the py2. The big things are the explicit calls to list() to exhaust the map() iterators, a consistent usage of text_type versus unicode (text_type had existed previously, but was inconsistently utilized), and a condtional import for urlparse(). Beyond this, there is one removal of some print statements that aren't currently reachable (these should have, imo, been click.echo calls anyway) and some style fixes that are strictly opinions on m part (you're using map much too heavily in this, even in places where a for-loop is much more readable.)

Feel free to merge the py3 compat fixes without merging the style fixes. I can also move those commits to another branch and open a new PR if you want to talk about them separately.

darvid commented 8 years ago

This is great, thanks. I'm fine with merging everything, although a couple notes before I do:

bbenne10 commented 8 years ago

With regard to six, I'd advocate for it. I thought about adding it in this PR, but I thought I'd start out with the smallest thing that could possibly fix the bug.

The use of list comprehensions over map calls is an improvement, imo, but I'd like to see most of them rewritten to use explicit for-loops. Particularly if they're nested._