Added CSVData tests for io streams #327
Conversation
gautomdas
requested review from
AnhTruong,
ChrisWallace2020,
grant-eden,
JGSweets and
lettergram
as code owners
| with self.assertRaisesRegex(ValueError, | ||
| '`header` must be one of following: auto, ' | ||
| 'none for no header, or a non-negative ' | ||
| 'integer for the row that represents the ' | ||
| 'header \(0 based index\)'): | ||
| csv_data = CSVData(filename, options=options) | ||
| first_value = csv_data.data.loc[0][0] | ||
|
|
||
| # set bad header setting | ||
| options = dict(header='abcdef') | ||
| with self.assertRaisesRegex(ValueError, | ||
| '`header` must be one of following: auto, ' | ||
| 'none for no header, or a non-negative ' | ||
| 'integer for the row that represents the ' | ||
| 'header \(0 based index\)'): | ||
| csv_data = CSVData(filename, options=options) | ||
| first_value = csv_data.data.loc[0][0] |
There was a problem hiding this comment.
We can remove the assertRaises checks as it is covered in the other test. Additionally, if we did want these tests, I would suggest not putting them in the for loop as we would only need to test it once.
gautomdas
force-pushed
the
csv_data_tests
branch
from
f0cf84e to
905c0b5
Compare
| input_file["path"]) | ||
|
|
||
| with open(input_file['path'], 'r', encoding=input_file['encoding']) as fp: | ||
| byte_string = StringIO(fp.read()) |
There was a problem hiding this comment.
not a blocker for approval, but byte_string maybe should be buffer or stream
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
I think this is fine for now, but I think we should refactor this immediately after to clean up tests and set the future precedent.
Essentially, wether it is read from the file or from the buffer, they should be identical meaning the tests can be used the same.
Something like this:
def setUpClass(cls):
.
.
.
cls.buffer_list = []
for input_file in cls.input_file_names:
#
# Create stream here
# buffer = ...
buffer_info = input_file.copy()
buffer_info['path'] = buffer
cls.buffer_list.append(buffer_info)
)
.
.
.
# then in tests also loop through the buffer list with existing tests not made specifically for streamsAgain, not saying it should be this PR. We can refactor directly after.
* Added CSVData tests for io streams * Change Data to CSVData * Fixed parameter issue and checked tests * Added change to fix 3.6 bug * Made small changes
Added the tests to compare CSVData functionality with streams