Fix csv2rec for passing in both names and comments. #4455
Conversation
| @@ -2867,14 +2867,19 @@ def get_func(name, item, func): | |||
| 'print': 'print_', | |||
| } | |||
|
|
|||
| def get_converters(reader): | |||
| def get_converters(reader, comments): | |||
There was a problem hiding this comment.
need to make comments optional. Perhaps defaults to an empty tuple (which is non-mutable, so it is safe). Should probably also have a line that coerces comments into a tuple since startswith() does not accept a list.
There was a problem hiding this comment.
comments is not treated as a list of chars but as a string in the whole csv2rec function.
There was a problem hiding this comment.
Ah, I just now notice that. And I also now see that this is not a public function, so comments doesn't need to be optional.
|
This also brings up the question of why we have this function at all. This should probably live in either numpy or pandas. |
|
@tacaswell, indeed, much more complicated and advanced functions do exist elsewhere. I also can guarantee you that someone will have found a way to use our reader for some type of file structure that no other reader can handle, which is why there are so many CSV readers. I do have one nitpick question. Should commented out and blank lines count towards the Assuming Travis passes, this looks good to me and fixes a legitimate bug. |
FIX: csv2rec for passing in both names and comments.
|
@WeatherGod Should we back-port this to color_overhaul |
|
I see it as a bugfix, so, yes. |
FIX: csv2rec for passing in both names and comments. fixed conflicts in mlab.py in favor of the incoming changes
|
|
Currently
csv2recfails when parsing a CSV file with a comment if one provides the column names.For example with a file test.txt containing:
Calling
csv2rec('test.txt', names=['a', 'b', 'c'])raises anIndexError.This PR patches it by honoring the
commentsargument passed tocsv2rec.