TST: Refactor test_parsers.py
State: closed

Refactored tests in `test_parsers.py` to increase coverage of the different types of parsers and remove nearly duplicate testing in some cases.


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO CSV
description: read_csv, to_csv

Comments:
Not sure if this is all the refactoring that needs to be done, but I think I got through a good portion of it.  Further suggestions are welcome!

changing things like this:

`# see gh-10022` are just not useful. you are the only one who uses this syntax AFAICT. leave them.

it is really quite difficult to tell what you did and comment on it. 

I would split this up into multiple files (`test_parsers`) to make this more easily grokable.

e.g.

`tests/parsers/....`

`# see gh-xxxx` is standard from other open source work I have done because it clearly labels that the issue was brought up on GitHub (as it may have migrated from other hosting systems as this one was for example).

Ah, yes, unfortunately, I did not realize how much I had changed until I saw that GitHub could not show the diff at all.  Also, your code block is blank if you were trying to demonstrate something...

@gfyoung what I was saying (I edited), was the `test_parsers` could/should just be splt up into multiple files under `pandas/io/tests/parsers/` (like did for things like `index/series/frame` etc. That way easier to read/grok/diff. Don't 'change' things as much as reorg, then in a follow-on you can change. Doing 2 at once is very confusing.

Hmmm...I'll see what I can do to break up the tests.  It's not as straightforward as it is in `frame.py` because of that `ParserTests` monolith.  Suggestions on how to / where to break it up (based on what you see in `master`)?

Also, this PR is pretty much just reorganization and removal of duplicate tests.  If you can view it, there are no more tests under `TestCParserHighMemory` or `TestCParserLowMemory`, as they have all been moved in `CParserTests` and `ParserTests`.

Also, one more thing: why do we have `test_cparser.py`?  That to me is very confusing when we already have `test_parsers.py`.

@gfyoung my point is this is very hard to see. Let's break up first. Yes no realy need for a separate `test_cparser.py`, those could be folded.

This IS straightforward to breakup if you just have different test classes rather than a single monolith. The idea is to test different options in different files. Perfect example is compressions which is alredy split out. But you can split out parse_date testing. line terminator testsing etc. Lots of ways to do this.

@jreback : No, I perfectly understand (I realized that it might be a problem when I saw GitHub couldn't show it)!  I was just trying to get some feedback first on how to break up `test_parsers.py`.  I'll see what I can see with this.

## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/12964?src=pr) is **83.92%**

> Merging [#12964](https://codecov.io/gh/pydata/pandas/pull/12964?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will decrease coverage by **-0.13%**

``` diff
@@             master     #12964   diff @@
==========================================
  Files           136        136          
  Lines         49994      49918    -76   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          42028      41889   -139   
- Misses         7966       8029    +63   
  Partials          0          0          
```
1. 2 files (not in diff) in `pandas/tools` were modified. [more](https://codecov.io/gh/pydata/pandas/commit/95c52b85ca2b671d5197c5341c5c30304d7f1263/changes?src=pr#70616E6461732F746F6F6C73) 
   - Hits `-22`
2. 9 files (not in diff) in `pandas/core` were modified. [more](https://codecov.io/gh/pydata/pandas/commit/95c52b85ca2b671d5197c5341c5c30304d7f1263/changes?src=pr#70616E6461732F636F7265) 
   - Misses `+66` 
   - Hits `-101`
3. 2 files (not in diff) in `pandas` were modified. [more](https://codecov.io/gh/pydata/pandas/commit/95c52b85ca2b671d5197c5341c5c30304d7f1263/changes?src=pr#70616E646173) 
   - Misses `-3` 
   - Hits `-16`

> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [84725fa...95c52b8](https://codecov.io/gh/pydata/pandas/compare/84725fa936b4136657f5ea02c588790de6866899...95c52b85ca2b671d5197c5341c5c30304d7f1263)

I think this also needs an edit to `setup.py`

e.g. add `pandas.io.tests.parsers` (also add `pandas.io.tests.sas`) which is missing. (`pandas.io.tests.json`) is already there.

you can do a 

`python setup.py sdist` to make sure that the tests are generated (when not from a clone)

@gfyoung I didn't want to you move any of the actual data files. They are used by other tests. and keeping in `io/tests/data` is fine. If there are parser specific ones then that would be ok to move (and are for sure not used anywhere else)

e.g. you moved a `gbq` one.

@jreback : Not true.  What I did was make a copy of the `data/` folder as  `parser/data/` and then took away anything that wasn't necessary.  FYI, the `gbq` one did not move.  That's a new copy of it.  Git(Hub) would have displayed the diff differently if it had been moved.

Also, Travis would have failed had I moved anything that was needed elsewhere.  I didn't copy the `data/` folder initially, and I in fact segfaulted some of the build machines.

no, gbq is not tested on travis (well only the inputs are).

leave `salary*` and `tips*` in the original location. you _can_ just create a soft-link if you want. The other moves are ok. (move `gbq` back as well). If you actually do need it to ensure parsing, then we should move that routine itself to the `test_gbq`

soft-link `iris` as well.

These are 'known' locations so don't really want to move them.

something wrong. lots of tests not running

```
[jreback-~/pandas] nosetests  pandas/io/tests/parser/
.............S...............S..........................
----------------------------------------------------------------------
Ran 56 tests in 15.089s

OK (SKIP=2)
```

but if its explict it works

```
[jreback-~/pandas] nosetests pandas/io/tests/parser/test_parsers.py 
...........................................S.........................................................................................................................................S............................................S.........................................................................................................................................S..S.............................................................................................................................................................S.
----------------------------------------------------------------------
Ran 527 tests in 47.291s

OK (SKIP=6)
```

I _think_ that `test_parsers.py`  permissions are messing this up, they should be 644 (but you have to force the checking)

```
[jreback-~/pandas] ls -ltr pandas/io/tests/*.py
-rw-rw-r--  1 jreback  staff    4757 Apr  5 19:04 pandas/io/tests/test_wb.py
-rw-rw-r--  1 jreback  staff    7936 Apr  5 19:04 pandas/io/tests/test_ga.py
-rw-rw-r--  1 jreback  staff    5892 Apr  5 19:04 pandas/io/tests/test_date_converters.py
-rw-rw-r--  1 jreback  staff    2755 Apr  5 19:04 pandas/io/tests/test_common.py
-rw-rw-r--  1 jreback  staff    4012 Apr  5 19:04 pandas/io/tests/test_clipboard.py
-rw-rw-r--  1 jreback  staff    9126 Apr  5 19:04 pandas/io/tests/generate_legacy_storage_files.py
-rw-rw-r--  1 jreback  staff       0 Apr  5 19:04 pandas/io/tests/__init__.py
-rw-rw-r--  1 jreback  staff   48615 Apr 27 09:48 pandas/io/tests/test_stata.py
-rw-rw-r--  1 jreback  staff    7505 Apr 27 09:48 pandas/io/tests/test_pickle.py
-rw-rw-r--  1 jreback  staff   20329 Apr 27 09:48 pandas/io/tests/test_data.py
-rw-rw-r--  1 jreback  staff  105456 Apr 28 13:21 pandas/io/tests/test_sql.py
-rw-rw-r--  1 jreback  staff   29769 Apr 28 13:21 pandas/io/tests/test_html.py
-rw-rw-r--  1 jreback  staff  202091 Apr 28 16:38 pandas/io/tests/test_pytables.py
-rw-rw-r--  1 jreback  staff   30648 Apr 28 16:38 pandas/io/tests/test_packers.py
-rw-rw-r--  1 jreback  staff   34349 Apr 28 16:38 pandas/io/tests/test_gbq.py
-rw-rw-r--  1 jreback  staff   84066 Apr 28 16:38 pandas/io/tests/test_excel.py
-rwxrwxr-x  1 jreback  staff  184802 Apr 29 12:59 pandas/io/tests/test_parsers.py
-rw-rw-r--  1 jreback  staff   12631 Apr 29 12:59 pandas/io/tests/test_cparser.py
```

@jreback : I think you're right about that (at least that's what Google says).  I changed the mode of `test_parsers.py` to 644.  That should work now hopefully.

I think need to add `io/tests/parser` to `ci/lint.sh` (and fix `common.py`). pls add `io/tests/sas` and `io/tsests/json` as well (ideally it would recursively descend .........)

`flake8` is recursive AFAICT.  I just tried doing `flake8 pandas/io --filename '*.py'` and it spewed out all of these errors with `common.py`, so adding them is not necessary.

`ci/lint.sh` is NOT though. we are specifically checking certain dirs. so we can change this to cover cases like this maybe.

Huh?  What do you mean?  That's the exact command `ci/lint.sh` will run.

so why didn't your build fail then? `common.py` is NOT checked (well not on travis).

I haven't pushed `common.py` to Travis yet.  I put `[ci skip]` explicitly for that reason.  I'm trying to see if I can extract some more test classes from that module before I push again to Travis.

oh, ok, then pls push. I think this is ready as is. (pls flake check)

@jreback : Travis is giving the green light.  Not sure what's going on with Codecov though.  Is this good to merge?

hmm, yeah the codecov that is looking odd

So how to proceed?  Can this be merged?  Or is it a bug in `codecov.yml`?  Something else?  Similar Codecov strangeness is also happening in my `fromnumeric` PR FYI.

yeah will take another look. if you have insite into codecov, then ok!