ENH: Better error message if usecols doesn't match columns
State: closed

GH17301: Improving the error message given when usecols
doesn't match with the columns provided.

- [x] closes #17301
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry 

NOTE: Do I need to add a whatsnew entry for something so small? 
  Happy to do so if needed.

Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: IO CSV
description: read_csv, to_csv

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=h1) Report
> Merging [#17310](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/2bec750b21b8715e3f55e71a6c69f2abef54d08b?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `50%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17310/graphs/tree.svg?height=150&width=650&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17310      +/-   ##
==========================================
- Coverage   91.01%   90.99%   -0.02%     
==========================================
  Files         162      162              
  Lines       49558    49560       +2     
==========================================
- Hits        45105    45097       -8     
- Misses       4453     4463      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.77% <50%> (-0.01%)` | :arrow_down: |
| #single | `40.25% <0%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.39% <50%> (-0.06%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.72% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=footer). Last update [2bec750...15d4786](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=h1) Report
> Merging [#17310](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/d163de70c93547035579870e2ae9008cb3640b45?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `81.81%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17310/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17310      +/-   ##
==========================================
- Coverage   91.46%   91.44%   -0.02%     
==========================================
  Files         157      157              
  Lines       51447    51455       +8     
==========================================
- Hits        47055    47053       -2     
- Misses       4392     4402      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.31% <81.81%> (-0.01%)` | :arrow_down: |
| #single | `40.57% <9.09%> (-0.12%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.55% <81.81%> (-0.05%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17310/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.81% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=footer). Last update [d163de7...5dfccdb](https://codecov.io/gh/pandas-dev/pandas/pull/17310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback - I've addressed your requested changes, anything else that's needed?
@gfyoung - Thanks for all of your help, please let me know if you need anything else. 😄 
Hey @gfyoung / @jreback, sorry for the delay.

I'm just looking at how I'd refactor these two functions (to refresh memory, that's the current `validate_usecols_arg` and the new, `validate_usecols` into one) and can't figure out a way to do it that wouldn't result in a mess.

Some of the stuff in the CParserWrapper relies on the output of `validate_usecols_arg` to work, while in the PythonParser, the point where we're calling it I don't think we have enough context to be able to drop the new logic in (all the logic in `_infer_columns` would need to be moved around I think?).

I'm really happy to do this if it's a must have for this to be accepted, but I'm afraid I'd need a little guidance on where/how to move things so I don't shoot in the dark and make things worse!

Thanks!
@AaronCritchley : The integration isn't so bad.  Just do the following per @jreback suggestion

~~~python
def _validate_usecols_arg(usecols, names=None):
     if names is None:
          # use original logic in _validate_usecols_arg
     else:
          # use your logic in _validate_usecols
~~~
Oh, so just two different branches of logic in that function? It feels a little weird that in some cases it would be returning two values, and in others returning none. If it's a design pattern you're happy for me to take then I can implement though
Python is not a declarative language, so we can do whatever we want. :smile: 

Write it out, and we can take a look afterwards.
Hey @jreback, would you like any further changes or is this good to go?
Ping @jreback - happy to make more changes if needed, would be great to get this in to master 😄 
@jreback Green 😄 
thanks @AaronCritchley 
I think these regexes need a ``r`` to make them raw; these warnings are on 3.6
```
DeprecationWarning: invalid escape sequence \[
  with tm.assert_raises_regex(ValueError, msg.format(missing="\['f'\]")):
/home/travis/build/pandas-dev/pandas/pandas/tests/io/parser/usecols.py:499: DeprecationWarning: invalid escape sequence \[
  with tm.assert_raises_regex(ValueError, msg.format(missing="\['f'\]")):
/home/travis/build/pandas-dev/pandas/pandas/tests/io/parser/usecols.py:504: DeprecationWarning: invalid escape sequence \[
  ValueError, msg.format(missing="\[('f', 'g'|'g', 'f')\]")):
/home/travis/build/pandas-dev/pandas/pandas/tests/io/parser/usecols.py:528: DeprecationWarning: invalid escape sequence \[
  with tm.assert_raises_regex(ValueError, msg.format(missing="\['f'\]")):
/home/travis/build/pandas-dev/pandas/pandas/tests/io/parser/usecols.py:532: DeprecationWarning: invalid escape sequence \[
  with tm.assert_raises_regex(ValueError, msg.format(missing="\['f'\]")):
```

https://travis-ci.org/pandas-dev/pandas/jobs/319713338

there are a bunch more as well.

@gfyoung 
cc @AaronCritchley 


Will look into this over the weekend, ty for the heads up!