Enable to switch off the removal of comments in csv2rec. #1699

Merged
merged 2 commits into from Jan 29, 2013

Conversation

Projects
None yet
2 participants
Contributor

gatagat commented Jan 24, 2013

This patch adds a possibility to pass comments=None to csv2rec in order to switch off comments removal.

A real world example are CSV files where the header on the first line is introduced by a hash #. As a hash is the default value for the comments option, the header is ignored. To read such files including the header one has to call csv2rec with something like comments='AStringThatNeverHappensToBeAtTheBeginingOfTheLine' which is not clear from the documentation and which seems more like a hack than a solution.

Currently, the docstring gives no hint about this behaviour and is misleading in the sense that it speaks about a character not a string:

- *comments*: the character used to indicate the start of a comment  in the file
lib/matplotlib/mlab.py
@@ -2109,7 +2109,7 @@ def csv2rec(fname, comments='#', skiprows=0, checkrows=0, delimiter=',',
files is automatic, if the filename ends in '.gz'
- *comments*: the character used to indicate the start of a comment
- in the file
+ in the file, or None to switch off the removal of comments
@WeatherGod

WeatherGod Jan 24, 2013

Member

Could you do *None* (asterisks around the word None), please? We are in the process of updating our docs to a consistent standard notation.

lib/matplotlib/mlab.py
@@ -2274,7 +2274,7 @@ def get_converters(reader):
if needheader:
for row in reader:
#print 'csv2rec', row
- if len(row) and row[0].startswith(comments):
+ if len(row) and comments != None and row[0].startswith(comments):
@WeatherGod

WeatherGod Jan 24, 2013

Member

Don't use equality (or inequality) with None's. Do "is" and "is not" instead.

lib/matplotlib/mlab.py
@@ -2317,7 +2317,7 @@ def get_converters(reader):
while 1:
# skip past any comments and consume one line of column header
row = next(reader)
- if len(row) and row[0].startswith(comments):
+ if len(row) and comments != None and row[0].startswith(comments):
@WeatherGod

WeatherGod Jan 24, 2013

Member

same here

lib/matplotlib/mlab.py
@@ -2327,7 +2327,7 @@ def get_converters(reader):
rowmasks = []
for i, row in enumerate(reader):
if not len(row): continue
- if row[0].startswith(comments): continue
+ if comments != None and row[0].startswith(comments): continue
@WeatherGod

WeatherGod Jan 24, 2013

Member

...and here. plus, while you are at it, could you put the "continue" on its own line, as per PEP8? Thank you.

Contributor

gatagat commented Jan 24, 2013

There is also the load function which has a comments option with the same behavior but I guess it does not make sense to change it as load() is deprecated (?).

Member

WeatherGod commented Jan 25, 2013

travis failures are likely duds. The code change is simple enough, and I guess it is reasonable. There is always the question of whether one should be able to ignore the comment symbol in the first line only, but that is starting to get complicated, and we are just a graphing library. I see no reason not to merge this as-is. I'll leave this up for the day in case anybody has any objections.

WeatherGod added a commit that referenced this pull request Jan 29, 2013

Merge pull request #1699 from gatagat/fix_csv2rec_comments
Enable to switch off the removal of comments in csv2rec.

@WeatherGod WeatherGod merged commit b69cc7c into matplotlib:master Jan 29, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment