Test Decorators and Better Pytest Integration in 'test_excel'
State: closed

- [x] closes #19816 
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Once I started looking at it, it made sense to make the module more "pytest compatible". There was a lot of skipping being done imperatively that seemed better suited to using class-level decorators, so as I moved some of that around I also replaced the setup/teardown mechanism of the "Excel Writing" tests with a pytest fixture

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: IO Excel
description: read_excel, to_excel

Comments:
The Travis network failures seem specific to Py27 (ran fine locally on Py3 but could reproduce with Py27). Assume that something is getting crossed up between the fixtures and the decorator for that test
So this is extremely nuanced but it looks like the combination of Py27, using `functools.wraps` and the `tm.network` decorator do not play nice together. I found another like issue open on Pytest's bug tracker and their suggestion was to use the `wraps` method from `six` to be backwards compatible with Py27.

Before making a change on that front I figure it's worth stopping here and getting feedback on the PR as is, but that again is one possible solution to the failures.

Here's the issue I mentioned:
https://github.com/pytest-dev/pytest/issues/2782

And the "offender" in the pandas source:
https://github.com/pandas-dev/pandas/blob/3b135c3c4424cfa10b955a0d505189f0a06e9122/pandas/util/testing.py#L2168
looks pretty reasonable
ok on fixing the wraps
thing can just add the six wraps inline in pandas.compat (or separate file of too big)
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19829?src=pr&el=h1) Report
> Merging [#19829](https://codecov.io/gh/pandas-dev/pandas/pull/19829?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/92dbc78af9f34af83691d86347fc04d2c543b94c?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `53.84%`.

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

```diff
@@            Coverage Diff             @@
##           master   #19829      +/-   ##
==========================================
- Coverage   91.66%   91.65%   -0.02%     
==========================================
  Files         150      150              
  Lines       48938    48949      +11     
==========================================
+ Hits        44860    44865       +5     
- Misses       4078     4084       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.03% <53.84%> (-0.02%)` | :arrow_down: |
| #single | `41.81% <53.84%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19829?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/19829/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `83.85% <100%> (ø)` | :arrow_up: |
| [pandas/util/\_test\_decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/19829/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX3Rlc3RfZGVjb3JhdG9ycy5weQ==) | `92.42% <100%> (+0.36%)` | :arrow_up: |
| [pandas/compat/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/19829/diff?src=pr&el=tree#diff-cGFuZGFzL2NvbXBhdC9fX2luaXRfXy5weQ==) | `57.62% <25%> (-1.15%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19829?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/19829?src=pr&el=footer). Last update [92dbc78...872e7d6](https://codecov.io/gh/pandas-dev/pandas/pull/19829?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

I think my comment got garbled. ok to fix the wraps things by using the six version. just include it in ``pandas.compat`` (or as a separate file if too big).
Can you check to make sure this doesn't hit the issue described in https://github.com/pytest-dev/pytest/issues/568?

I don't recall the details, but it was something to do with applying skip marks to child classes causing parents & or siblings to skip, which sounds similar to what's changing here.
@TomAugspurger thanks for sharing. Fortunately that issue should not affect this design here, as the only subclasses with class-level `skip_if` decorators inherit `_WriterBase`, which doesn't have any tests it needs to execute on its own. 

FWIW I compare the before / after results with various combinations of these packages installed in a virtual environment and the numbers matched in all cases, save the fact that there are 3 new tests as a result of parametrizing `test_ExcelWriter_dispatch` and creating `test_ExcelWriter_dispatch_raises`
Great, thanks for checking.

On Thu, Feb 22, 2018 at 10:21 AM, William Ayd <notifications@github.com>
wrote:

> @TomAugspurger <https://github.com/tomaugspurger> thanks for sharing.
> Fortunately that issue should not affect this design here, as the only
> subclasses with class-level skip_if decorators inherit _WriterBase, which
> doesn't have any tests it needs to execute on its own.
>
> FWIW I compare the before / after results with various combinations of
> these packages installed in a virtual environment and the numbers matched
> in all cases, save the fact that there are 3 new tests as a result of
> parametrizing test_ExcelWriter_dispatch and creating
> test_ExcelWriter_dispatch_raises
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/19829#issuecomment-367734770>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIvPvSYJhOiVkTr7IwaF8rZDLk2peks5tXZQAgaJpZM4SOXaZ>
> .
>

I removed xlwt (only) and got this:

```================================================================================================================ FAILURES ================================================================================================================
______________________________________________________________________________________ TestExcelWriter.test_to_excel_timedelta[openpyxl-.xlsx-True] ______________________________________________________________________________________
pandas/tests/io/test_excel.py:1386: in test_to_excel_timedelta
    frame.to_excel(path, 'test1')
pandas/core/frame.py:1599: in to_excel
    engine=engine)
pandas/io/formats/excel.py:646: in write
    writer = ExcelWriter(_stringify_path(writer), engine=engine)
pandas/io/excel.py:1432: in __init__
    import xlwt
E   ModuleNotFoundError: No module named 'xlwt'
_____________________________________________________________________________________ TestExcelWriter.test_to_excel_timedelta[openpyxl-.xlsx-False] ______________________________________________________________________________________
pandas/tests/io/test_excel.py:1386: in test_to_excel_timedelta
    frame.to_excel(path, 'test1')
pandas/core/frame.py:1599: in to_excel
    engine=engine)
pandas/io/formats/excel.py:646: in write
    writer = ExcelWriter(_stringify_path(writer), engine=engine)
pandas/io/excel.py:1432: in __init__
    import xlwt
E   ModuleNotFoundError: No module named 'xlwt'
_____________________________________________________________________________________ TestExcelWriter.test_to_excel_timedelta[xlsxwriter-.xlsx-True] _____________________________________________________________________________________
pandas/tests/io/test_excel.py:1386: in test_to_excel_timedelta
    frame.to_excel(path, 'test1')
pandas/core/frame.py:1599: in to_excel
    engine=engine)
pandas/io/formats/excel.py:646: in write
    writer = ExcelWriter(_stringify_path(writer), engine=engine)
pandas/io/excel.py:1432: in __init__
    import xlwt
E   ModuleNotFoundError: No module named 'xlwt'
____________________________________________________________________________________ TestExcelWriter.test_to_excel_timedelta[xlsxwriter-.xlsx-False] _____________________________________________________________________________________
pandas/tests/io/test_excel.py:1386: in test_to_excel_timedelta
    frame.to_excel(path, 'test1')
pandas/core/frame.py:1599: in to_excel
    engine=engine)
pandas/io/formats/excel.py:646: in write
    writer = ExcelWriter(_stringify_path(writer), engine=engine)
pandas/io/excel.py:1432: in __init__
    import xlwt
E   ModuleNotFoundError: No module named 'xlwt'
=========================================================================================== 4 failed, 293 passed, 116 skipped in 13.11 seconds ===========================================================================================
```
Just pushed updates. FWIW there was an error in the last Circle / Travis builds for `TestExcelWriter.test_to_excel_timedelta`. That test that originally hard-coded to use '.xls' files so was never testing for `openpyxl` and `xlsxwriter`. I'm imperatively skipping the former as that was not working and needs to be addressed separately (see #19900), but the later seems to work for *most* configurations.

I can't reproduce the issue locally on OSX with Py27 so perhaps it's a Linux-only issue? Waiting to see what happens this time around, but assuming it pops up again I would plan on imperatively skipping for the combination of Py27 and Linux for now and opening a separate issue to address that

<details>

INSTALLED VERSIONS
------------------
commit: 849933d2c2876d7e7bac3e44969cf27eb82c9e1f
python: 2.7.14.final.0
python-bits: 64
OS: Darwin
OS-release: 17.4.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: None.None

pandas: 0.23.0.dev0+400.g849933d2c
pytest: 3.2.3
pip: 9.0.1
setuptools: 36.6.0
Cython: 0.27.3
numpy: 1.13.3
scipy: None
pyarrow: None
xarray: None
IPython: 5.5.0
sphinx: None
patsy: None
dateutil: 2.6.1
pytz: 2017.3
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: 2.5.0
xlrd: 1.1.0
xlwt: 1.3.0
xlsxwriter: 1.0.2
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: None
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>
happy to hve in this one 
but if it is holding this back can do in a follow up
lmk
Tried locally and everything passed with '.xlsm' so I'd prefer to include in this. Will re-push after seeing output of latest commit in Travis/CircleCI
thanks @WillAyd keep em coming!

and ideally if you can submit PR's for the followups you noted (I think you created issues for all)