Python 3 support #9
Conversation
tox.ini
Outdated
| @@ -1,7 +1,8 @@ | |||
| [tox] | |||
| minversion = 2.4 | |||
| envlist = py{2.7,3.6} | |||
There was a problem hiding this comment.
I'd rather not specify the Python versions here, as some environments may not have these versions. Let's just specify the target versions in Travis and let tox run against the local python version.
There was a problem hiding this comment.
Fair enough; I guess this requires appending - 3.6 to python: in .travis.yml then?
There was a problem hiding this comment.
Fixed, hopefully Travis will pick it up
tox.ini
Outdated
|
|
||
| [testenv] | ||
| changedir = test |
There was a problem hiding this comment.
Do the tests work without this change (keeping usedevelop)? I've found usedevelop is necessary for some packages, and I'd rather diverge as little as possible from the scaffolding that other projects need.
There was a problem hiding this comment.
I will check.
// Doing in-place tests is usually a bad idea since it doesn't test that you've actually packaged things properly (or messed up the imports), since your code will not be imported the same way as the end users will do it. Most often than not, changedir to a folder that doesn't contain __init__.py isolates the test code from the the source tree and ensures you're testing against site-packages.
There was a problem hiding this comment.
Interesting point. I do agree that's a risk... and perhaps your technique is indeed superior. I do struggle a little bit with it for several reasons: (1) not all projects have a 'tests' directory, so to generalize the technique, one would need to create this directory. (2) pytest would only discover tests in this directory, meaning all tests would need to be moved to this directory, which isn't even possible for some tests like doctests and (3) When running tests against installed packages, they end up getting long, bulky paths in the traceback instead of the nearby local paths fuzzy vs .tox/envname/lib/pythonN.N/site-packages/fuzzy (maybe even absolute).
There was a problem hiding this comment.
Fair enough, I've reverted it for now.
test/test_fuzzy.py
Outdated
| import ctypes | ||
|
|
||
| import fuzzy | ||
|
|
||
| def test_soundex_does_not_mutate_strings(): | ||
| phrase = 'FancyFree' | ||
| phrase = u'FancyFree' |
There was a problem hiding this comment.
u'' syntax is unnecessary and superfluous when unicode_literals is present.
| @@ -1,1657 +0,0 @@ | |||
| /* Generated by Pyrex 0.9.8.6 on Sat Feb 4 15:03:10 2012 */ | |||
There was a problem hiding this comment.
While it's nice to omit the generated code, it's necessary to create sdists that can be installed without Cython, which presumably was a goal at one point. I'm happy to omit this file, but we should be explicit about which installations this might break and how to mitigate the effects.
Fixes #7.