Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix csv2rec for passing in both names and comments. #4455

Merged
merged 3 commits into from May 22, 2015
Merged

Fix csv2rec for passing in both names and comments. #4455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90942c9
Fix csv2rec for passing in both names and comments.
gatagat May 22, 2015
1c7c598
Patch wrong checkrows.
gatagat May 22, 2015
634de83
fix
gatagat May 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions lib/matplotlib/mlab.py
View file
Open in desktop
Expand Up @@ -2867,14 +2867,20 @@ def get_func(name, item, func):
'print': 'print_',
}

def get_converters(reader):
def get_converters(reader, comments):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments is not treated as a list of chars but as a string in the whole csv2rec function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


converters = None
for i, row in enumerate(reader):
i = 0
for row in reader:
if (len(row) and comments is not None and
row[0].startswith(comments)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, we are only checking a single character, so maybe it might make more sense to do row[0] in comments, which would eliminate the tuple restriction noted above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do rows come out stripped? Do we want # comment to also be a comment line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the same test is used in the main loop of csv2rec. So unless we change it everywhere (which might break things), I would leave it as it is.

continue
if i == 0:
converters = [mybool]*len(row)
if checkrows and i > checkrows:
break
i += 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have a blank line after this for readability?


for j, (name, item) in enumerate(zip(names, row)):
func = converterd.get(j)
if func is None:
Expand Down Expand Up @@ -2925,7 +2931,7 @@ def get_converters(reader):
names = [n.strip() for n in names.split(',')]

# get the converter functions by inspecting checkrows
converters = get_converters(reader)
converters = get_converters(reader, comments)
if converters is None:
raise ValueError('Could not find any valid data in CSV file')

Expand Down
7 changes: 7 additions & 0 deletions lib/matplotlib/tests/test_mlab.py
View file
Open in desktop
Expand Up @@ -339,6 +339,13 @@ def test_rec2csv_bad_shape_ValueError(self):
# the bad recarray should trigger a ValueError for having ndim > 1.
assert_raises(ValueError, mlab.rec2csv, bad, self.fd)

def test_csv2rec_names_with_comments(self):
self.fd.write('# comment\n1,2,3\n4,5,6\n')
self.fd.seek(0)
array = mlab.csv2rec(self.fd, names='a,b,c')
assert len(array) == 2
assert len(array.dtype) == 3


class window_testcase(CleanupTestCase):
def setUp(self):
Expand Down