Make Fabulous Python 2/3 compatible

jart / fabulous

Print images, colors, and stylish text to the terminal with Python

Apache License 2.0

347 stars 34 forks source link

Make Fabulous Python 2/3 compatible #12

Closed jart closed 8 years ago

jart commented 8 years ago

PTAL @willzfarmer @willtheorangeguy

TheDataLeek commented 8 years ago

Cool, this looks good, thanks for putting it together! I think it totally solves #11 and I can't wait to use it.

jart commented 8 years ago

Would it be too much trouble to ask that you pull this branch locally and try it out?

willtheorangeguy commented 8 years ago

Looks fine to me!

TheDataLeek commented 8 years ago

Nope, hold on, will check it asap.

TheDataLeek commented 8 years ago

Issues found:

The demo code needs to be updated to py3
Some of the tests need to be updated as well, for instance in fabulous/test_transientlogging.py:49,63

# change from
# This            to     this
happy.next()   |   next(happy)

I also am not able to run the tests (this may be a problem on my end...). It complains that there's no terminate module...

I'll keep digging for ya.

jart commented 8 years ago

Great finds!

I run the tests the same way the .travis.yml config runs them, which is currently only doctest. I need to update it to run that test_transientlogging.py file.

As for those files that use terminate, I'm not sure what those files are exactly. I accepted those as an external contribution a while ago. So I wasn't able to fully verify I updated them. I'm honestly just considering removing them, since some of the functionality they offers appears to overlap with other fabulous modules. Plus the fact that they require an additional dependency.

TheDataLeek commented 8 years ago

Ahhh, I get it. At one point this library was called terminate, and then the name updated but these tests didn't...

If you import fabulous instead of import terminate it works.

Issues found with using those tests

Update keys()

tests/term_test.py
>       for color in Magic.COLORS.iterkeys():
E       AttributeError: 'dict' object has no attribute 'iterkeys'

Another transientlogger bug. Update the behavior of range and lists. Fixed is below.

# in py3 range(n) returns generator, not list
for n in list(range(len(msg))) + list(reversed(range(len(msg)))):

There are a couple more generator next() issues through that transientlogger file that will need to be updated.

TheDataLeek commented 8 years ago

Also, once you update the imports in tests/ they all pass (for better or worse)

arielb2 commented 8 years ago

Great, I was also working on making fabulous compatible with python3 :) but very well that you did. thanks!. I tested your changes:

$ python3 setup.py install
$ fabulous-demo

and it worked well.

I will wait for the release of branch in order to continue with the upgrade package.

jart commented 8 years ago

Thanks for the help guys!