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

ENH: Better error message if usecols doesn't match columns #17310

Merged
merged 12 commits into from
Dec 3, 2017
Merged

ENH: Better error message if usecols doesn't match columns #17310

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b0e102a
ENH: better error message if usecols doesn't match
AaronCritchley Aug 22, 2017
15d4786
WIP: Changing Python parser behaviour, better tests
AaronCritchley Oct 4, 2017
841a6cc
WIP: Implementing suggested changes
AaronCritchley Oct 4, 2017
dced1b7
Fixing silly error in tests
AaronCritchley Oct 4, 2017
5bf89a8
Better documentation as per @gfyoung
AaronCritchley Oct 4, 2017
ba93833
Changing ValueError documentation
AaronCritchley Oct 5, 2017
8a06cee
Reverting, changing name of function
AaronCritchley Nov 8, 2017
1afb4c1
Merge branch 'master' into enhancement/better-usecols-message
AaronCritchley Nov 28, 2017
5a8a852
Merge branch 'master' into enhancement/better-usecols-message
AaronCritchley Dec 2, 2017
2209eae
Adding whatsnews entry, and test for multiple missing columns
AaronCritchley Dec 2, 2017
93185f5
Adding accidentally removed test back
AaronCritchley Dec 2, 2017
5dfccdb
Suggested whatsnew change
AaronCritchley Dec 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions pandas/io/parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,20 @@ def _evaluate_usecols(usecols, names):
return usecols


def _validate_usecols(usecols, names):
"""
Validates that all usecols are present in a given
list of names, if not, raise a ValueError that
Copy link
Member

Choose a reason for hiding this comment

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

Nit: let's remove the comma splice in this doc-string i.e.

"...names. If not, raise a ValueError that..."

Copy link
Member

Choose a reason for hiding this comment

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

Also, let's add a parameter description + return usecols (just in case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & done!

I've used the doc format of the existing functions that were below, if this needs to change let me know and I'll do so.

shows what usecols are missing.
"""
missing = [c for c in usecols if c not in names]
if len(missing) > 0:
raise ValueError(
"Usecols do not match columns, "
"columns expected but not found: {missing}".format(missing=missing)
Copy link
Contributor

Choose a reason for hiding this comment

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

you might need to ', '.join(missing) here, not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Python 3.6.3 |Anaconda, Inc.| (default, Nov  8 2017, 18:10:31) 
[GCC 4.2.1 Compatible Clang 4.0.1 (tags/RELEASE_401/final)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> l = [1, 'foo', [2,3]]
>>> 'Formatted l: {}'.format(l)
"Formatted l: [1, 'foo', [2, 3]]"

if you'd prefer a different error message, I'd be happy to use join though.

Copy link
Contributor

Choose a reason for hiding this comment

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

that’s fine i guess format is pretty smart about this ok!

)


def _validate_skipfooter_arg(skipfooter):
"""
Validate the 'skipfooter' parameter.
Expand Down Expand Up @@ -1662,14 +1676,14 @@ def __init__(self, src, **kwds):
# GH 14671
if (self.usecols_dtype == 'string' and
not set(usecols).issubset(self.orig_names)):
raise ValueError("Usecols do not match names.")
_validate_usecols(usecols, self.orig_names)

if len(self.names) > len(usecols):
self.names = [n for i, n in enumerate(self.names)
if (i in usecols or n in usecols)]

if len(self.names) < len(usecols):
raise ValueError("Usecols do not match names.")
_validate_usecols(usecols, self.names)

self._set_noconvert_columns()

Expand Down Expand Up @@ -2442,9 +2456,13 @@ def _handle_usecols(self, columns, usecols_key):
raise ValueError("If using multiple headers, usecols must "
"be integers.")
col_indices = []

for col in self.usecols:
if isinstance(col, string_types):
col_indices.append(usecols_key.index(col))
try:
col_indices.append(usecols_key.index(col))
except ValueError:
_validate_usecols(self.usecols, usecols_key)
else:
col_indices.append(col)
else:
Expand Down
16 changes: 8 additions & 8 deletions pandas/tests/io/parser/usecols.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,10 @@ def test_raise_on_usecols_names_mismatch(self):
# GH 14671
data = 'a,b,c,d\n1,2,3,4\n5,6,7,8'

if self.engine == 'c':
msg = 'Usecols do not match names'
else:
msg = 'is not in list'
msg = (
"Usecols do not match columns, "
"columns expected but not found: {missing}"
)

usecols = ['a', 'b', 'c', 'd']
df = self.read_csv(StringIO(data), usecols=usecols)
Expand All @@ -492,11 +492,11 @@ def test_raise_on_usecols_names_mismatch(self):
tm.assert_frame_equal(df, expected)

usecols = ['a', 'b', 'c', 'f']
with tm.assert_raises_regex(ValueError, msg):
with tm.assert_raises_regex(ValueError, msg.format(missing=['f'])):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we're repeating the same format for every scenario here, which seems silly, but if we were to extend this in the future it'd be useful to have this around (although trivial to implement if not) - would you rather me just have a string to check against and not bother formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the error I'm getting during testing is super confusing:

E           AssertionError: "Usecols do not match columns, columns expected but not found: ['f']" does not match "Usecols do not match columns, columns expected but not found: ['f']"

Am I being silly? Is it to do with the regex matching?

Copy link
Member

Choose a reason for hiding this comment

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

  • Let's see if we do the string formatting. We need to check that it raises on the right columns.
  • Yes, it's regex matching. So you'll need to add forward-slashes to the brackets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry on the error regex matching, that's me being silly, thanks!

self.read_csv(StringIO(data), usecols=usecols)

usecols = ['a', 'b', 'f']
with tm.assert_raises_regex(ValueError, msg):
with tm.assert_raises_regex(ValueError, msg.format(missing=['f'])):
self.read_csv(StringIO(data), usecols=usecols)

names = ['A', 'B', 'C', 'D']
Expand All @@ -520,9 +520,9 @@ def test_raise_on_usecols_names_mismatch(self):
# tm.assert_frame_equal(df, expected)

usecols = ['A', 'B', 'C', 'f']
with tm.assert_raises_regex(ValueError, msg):
with tm.assert_ra(ValueError, msg.format(missing=['f'])):
Copy link
Member

Choose a reason for hiding this comment

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

Hmm...what happened here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! Thank you!

self.read_csv(StringIO(data), header=0, names=names,
usecols=usecols)
usecols = ['A', 'B', 'f']
with tm.assert_raises_regex(ValueError, msg):
with tm.assert_raises_regex(ValueError, msg.format(missing=['f'])):
self.read_csv(StringIO(data), names=names, usecols=usecols)