Cleanup and 3.x compatibility

[chsamlee]

Changes:

• make the module python3-compatible
• proper implementation of __eq__ and __repr__
• change LispError to be subclass of LispDict (reason: a lispified error is also a plist, and subclassing allows some duplicate code to be removed)

Fixes #2 and unblocks #3.

[codecov-io]

Current coverage is 91.66% (diff: 92.59%)

Merging #5 into master will increase coverage by 2.13%

@@             master         #5   diff @@
==========================================
  Files             2          2          
  Lines           172        156    -16   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            154        143    -11   
+ Misses           18         13     -5   
  Partials          0          0          

Powered by Codecov. Last update cd70371...b7b3b85

[michaelsilver]

I'd be nice to have travis build across all versions of python:

diff --git a/.travis.yml b/.travis.yml
index 17306a6..116b0a4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,13 @@
 language: python
 python:
+  - "2.6"
   - "2.7"
+  - "3.2"
+  - "3.3"
+  - "3.4"
+  - "3.5"
+  - "3.5-dev" # 3.5 development branch
+  - "nightly" # currently points to 3.6-dev
 install:
   - pip install codecov pep8
   - python setup.py install

[chsamlee]

I don't think we should support 3.2 because very few people use it and 3.2 compatibility is more complicated: the u'unicode string' was removed in 3.0 and only readded in 3.3.

Given that travis relies on volunteers for computing resources, I don't think we should have a build for every version. I think 2.7, 3.3, and nightly should be enough, and maybe 2.6 if that is necessary.

[michaelsilver]

I don't think we should support 3.2 because very few people use it and 3.2 compatibility is more complicated: the u'unicode string' was removed in 3.0 and only readded in 3.3.

Ok, that sounds reasonable. Why don't you update the README with a list of supported Python versions so that it's clear.

Given that travis relies on volunteers for computing resources

I wouldn't worry about that too much. Millions of projects use Travis, and us using a few more builds is not going to make a difference.

I think 2.7, 3.3, and nightly should be enough, and maybe 2.6 if that is necessary.

Given my above reasoning, how about 2.6, 3.3, 3.4, 3.5, 3.5-dev, and nightly? I think 2.6 might not have codecov. If that's the case, omit the codecov step. I'm not too keen on 2.6, so if it's causing lots of problems, don't worry about it. But the other versions I listed I think we should try to support if possible.

[michaelsilver]

Awesome job! I'm so glad you figured this out, because I had significant trouble figuring out the details of unicode compatibility across Python 2 and 3. 🎉

In addition to my inline notes, could you please add some documentation to the lispify function explaining what kind of string it returns (depending on the Python version)? I noticed you were wrapping calls to the lispify function with unicode() and str() in the unit tests, but lots of other libraries that depend on lispify just call it bare, without wrapping. So it needs to be obvious to other developers what they get raw out of the lispify function.

[chsamlee]

The behavior of lispify is not changed in this PR, so other users shouldn't have to change their coe. It is the changes in __eq__ that requires wrapping lispify calls in unit tests - the current implementation treats a string s and a LispType object as equal if the object's string representation is same as s. Convenient for testing, but a bad practice regardless.

[michaelsilver]

It is the changes in __eq__ that requires wrapping lispify calls in unit tests - the current implementation treats a string s and a LispType object as equal if the object's string representation is same as s. Convenient for testing, but a bad practice regardless.

Hmm... I wonder whether there's a way to use a 3rd party library, or the __future__ import to solve this? Alternatively, maybe you could create a wrapper testing function to deal with the str conversion and do a type check?

[michaelsilver]

delete

[michaelsilver]

Other than deleting the accidentally left >>>>>>> master (see in-line note), LGTM! 🚀

(just delete it, and merge, no need to re-review)