Python2 compat#2
Conversation
in test code for 2.6 compatibility
by field name, no empty {}
|
I have another branch, oauth2client-token, that is pre-merged with this python2-compat branch. I can create a PR for that branch for you if you accept and merge this Pytonn2 -compatibilty PR. |
|
Also, I reference the travis-ci links in the README.rst, but github does not support any referencing of the current branch, only hard links. So I've added a commit to this PR that references your master branch and not mine. |
| @@ -0,0 +1,4 @@ | |||
| import sys, types | |||
| sys.modules['pandas'] = types.ModuleType('pandas', 'Fake pandas module') | |||
There was a problem hiding this comment.
All tests pass and everything looks good, but can you explain why you're mocking out the pandas module? Even if it isn't required right now, shouldn't we still test against pandas?
There was a problem hiding this comment.
Well, I do not wish to attempt to get pandas to build on the travis-ci host. 1) Testing a collaborative open source package against multiple Python versions requires something like travis-ci, and 2) using travis-ci means we have to build each test environment from scratch for each build. There are ways to retain assets from build to build on a TravisCI instance, but it requires a paid subscription.
There was a problem hiding this comment.
Also, note that pandas is a requirement in setup.py UNLESS the environment indicates it's travis-ci or another CI. So installing and running the tests locally will still check for pandas requirement, and import the genuine pandas module.
If the test code ever grows tests that actually call pandas code, our choices seem to be:
- Stop using travis-ci, which will make support for multiple Python versions hard to maintain.
- Upgrade to a paid travis-ci account, or wait until Travis offers the dependency caching feature to open-source projects.
- Mock the portions of the pandas module/contents that are covered by the tests, and use those mocks only in CI mode.
I would vote for #3.
|
Still interested in merging this PR @henrystokeley ? |
2 participants
Work required was annoying but it's complete, and doesn't complicate any future development for pgsheets.
I added travis-ci config and used travis-ci to confirm the new python 2.6 and 2.7 compatibility: https://travis-ci.org/robin900/pgsheets/builds/117465021
Feel free to set up your own travis-ci free account and let the travis builds do their thing.