Large PR -- Adding Tests, fixing a few bugs that revealed, and cleaning up small nits#2
Merged
benlower merged 7 commits intotableau:mastertableau/document-api-python:masterfrom May 12, 2016
t8y8:masterCopy head branch name to clipboard
Merged
Large PR -- Adding Tests, fixing a few bugs that revealed, and cleaning up small nits#2benlower merged 7 commits intotableau:mastertableau/document-api-python:masterfrom t8y8:masterCopy head branch name to clipboard
benlower merged 7 commits intotableau:mastertableau/document-api-python:masterfrom
t8y8:masterCopy head branch name to clipboard
Conversation
…file and add some test cases for a help method I plan to update in the next commit. I made _is_valid_file a static method since it's just a helper and it makes it easier to test
…more filetypes in the future. No more C++ code in the python :)
… code more readable. Also remove the unnecessary copy.copy call and import. Small refactor of parameter name for Workbook.save_as method to be more descriptive than 'value'. Manual testing on python2 and pyhon3 for example. Verified still works
…tion classes. The temp-file setUp isn't the greatest but it's functional for now and will help with confidance if we decide to move anything around. Already uncovered a bug with them, fixed in this commit -- TDS files don't seem to have a 'name' attribute but instead call it 'formatted-name' I added a simple OR into the _name logic of the Datasource class and that resolves the bug.
Document API/test.py
Outdated
| class DatasourceModelTests(unittest.TestCase): | ||
|
|
||
| def setUp(self): | ||
| self.tds_file = tempfile.NamedTemporaryFile(suffix='.tds') |
Contributor
Author
There was a problem hiding this comment.
My tests didn't work on Windows due to the way Windows handles the namedtemporaryfile.
I manually manage the files now and it works cross platform
…managing temp files right now, but we may want to modify our Workbook and Datasource classes to accept all File-Like objects in the future so I don't have to do this filename sillyness. Still, this works for now and is cross platform
Contributor
|
merging this. let's talk about the 'file-like' objects comment soon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sorry, this PR grew, I'm trying to remember my git-fu.
I'll start putting changes into their own branches and PRs :)
Changes:
Tests pass before and after change on python2 and python3