From 0a3251961527a3380243af0e3978e28757934e63 Mon Sep 17 00:00:00 2001
From: Simon Hawkins <simonjayhawkins@gmail.com>
Date: Sun, 30 Jun 2019 07:49:04 +0100
Subject: [PATCH] TST/CLN: engine fixture for tests/io/excel/test_readers.py

---
 pandas/tests/io/excel/test_readers.py | 53 ++++++++++++---------------
 pandas/tests/io/excel/test_writers.py | 24 +++++-------
 2 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/pandas/tests/io/excel/test_readers.py b/pandas/tests/io/excel/test_readers.py
index 40a6970aa7f04..be5951fe12b46 100644
--- a/pandas/tests/io/excel/test_readers.py
+++ b/pandas/tests/io/excel/test_readers.py
@@ -31,24 +31,29 @@ def ignore_xlrd_time_clock_warning():
         yield
 
 
+@pytest.fixture(params=[
+    # Add any engines to test here
+    pytest.param('xlrd', marks=td.skip_if_no('xlrd')),
+    pytest.param('openpyxl', marks=td.skip_if_no('openpyxl')),
+    pytest.param(None, marks=td.skip_if_no('xlrd')),
+])
+def engine(request):
+    """
+    A fixture for Excel reader engines.
+    """
+    return request.param
+
+
 class TestReaders:
 
-    @pytest.fixture(autouse=True, params=[
-        # Add any engines to test here
-        pytest.param('xlrd', marks=pytest.mark.skipif(
-            not td.safe_import("xlrd"), reason="no xlrd")),
-        pytest.param('openpyxl', marks=pytest.mark.skipif(
-            not td.safe_import("openpyxl"), reason="no openpyxl")),
-        pytest.param(None, marks=pytest.mark.skipif(
-            not td.safe_import("xlrd"), reason="no xlrd")),
-    ])
-    def cd_and_set_engine(self, request, datapath, monkeypatch, read_ext):
+    @pytest.fixture(autouse=True)
+    def cd_and_set_engine(self, engine, datapath, monkeypatch, read_ext):
         """
         Change directory and set engine for read_excel calls.
         """
-        if request.param == 'openpyxl' and read_ext == '.xls':
+        if engine == 'openpyxl' and read_ext == '.xls':
             pytest.skip()
-        func = partial(pd.read_excel, engine=request.param)
+        func = partial(pd.read_excel, engine=engine)
         monkeypatch.chdir(datapath("io", "data"))
         monkeypatch.setattr(pd, 'read_excel', func)
 
@@ -726,23 +731,15 @@ def test_read_excel_squeeze(self, read_ext):
 
 class TestExcelFileRead:
 
-    @pytest.fixture(autouse=True, params=[
-        # Add any engines to test here
-        pytest.param('xlrd', marks=pytest.mark.skipif(
-            not td.safe_import("xlrd"), reason="no xlrd")),
-        pytest.param('openpyxl', marks=pytest.mark.skipif(
-            not td.safe_import("openpyxl"), reason="no openpyxl")),
-        pytest.param(None, marks=pytest.mark.skipif(
-            not td.safe_import("xlrd"), reason="no xlrd")),
-    ])
-    def cd_and_set_engine(self, request, datapath, monkeypatch, read_ext):
+    @pytest.fixture(autouse=True)
+    def cd_and_set_engine(self, engine, datapath, monkeypatch, read_ext):
         """
         Change directory and set engine for ExcelFile objects.
         """
-        if request.param == 'openpyxl' and read_ext == '.xls':
+        if engine == 'openpyxl' and read_ext == '.xls':
             pytest.skip()
 
-        func = partial(pd.ExcelFile, engine=request.param)
+        func = partial(pd.ExcelFile, engine=engine)
         monkeypatch.chdir(datapath("io", "data"))
         monkeypatch.setattr(pd, 'ExcelFile', func)
 
@@ -830,20 +827,18 @@ def test_sheet_name(self, read_ext, df_ref):
         tm.assert_frame_equal(df1_parse, df_ref, check_names=False)
         tm.assert_frame_equal(df2_parse, df_ref, check_names=False)
 
-    def test_excel_read_buffer(self, read_ext):
+    def test_excel_read_buffer(self, engine, read_ext):
         pth = 'test1' + read_ext
-        engine = pd.ExcelFile.keywords['engine']  # TODO: fixturize
         expected = pd.read_excel(pth, 'Sheet1', index_col=0, engine=engine)
 
         with open(pth, 'rb') as f:
             with pd.ExcelFile(f) as xls:
                 actual = pd.read_excel(xls, 'Sheet1', index_col=0)
 
-            tm.assert_frame_equal(expected, actual)
+        tm.assert_frame_equal(expected, actual)
 
-    def test_reader_closes_file(self, read_ext):
+    def test_reader_closes_file(self, engine, read_ext):
         f = open('test1' + read_ext, 'rb')
-        engine = pd.ExcelFile.keywords['engine']  # TODO: fixturize
         with pd.ExcelFile(f) as xlsx:
             # parses okay
             pd.read_excel(xlsx, 'Sheet1', index_col=0, engine=engine)
diff --git a/pandas/tests/io/excel/test_writers.py b/pandas/tests/io/excel/test_writers.py
index ffa77de930cbd..d65bebe16804c 100644
--- a/pandas/tests/io/excel/test_writers.py
+++ b/pandas/tests/io/excel/test_writers.py
@@ -224,7 +224,7 @@ def test_read_excel_parse_dates(self, ext):
 class _WriterBase:
 
     @pytest.fixture(autouse=True)
-    def set_engine_and_path(self, request, engine, ext):
+    def set_engine_and_path(self, engine, ext):
         """Fixture to set engine and open file for use in each test case
 
         Rather than requiring `engine=...` to be provided explicitly as an
@@ -252,14 +252,10 @@ class and any subclasses, on account of the `autouse=True`
 
 @td.skip_if_no('xlrd')
 @pytest.mark.parametrize("engine,ext", [
-    pytest.param('openpyxl', '.xlsx', marks=pytest.mark.skipif(
-        not td.safe_import('openpyxl'), reason='No openpyxl')),
-    pytest.param('openpyxl', '.xlsm', marks=pytest.mark.skipif(
-        not td.safe_import('openpyxl'), reason='No openpyxl')),
-    pytest.param('xlwt', '.xls', marks=pytest.mark.skipif(
-        not td.safe_import('xlwt'), reason='No xlwt')),
-    pytest.param('xlsxwriter', '.xlsx', marks=pytest.mark.skipif(
-        not td.safe_import('xlsxwriter'), reason='No xlsxwriter'))
+    pytest.param('openpyxl', '.xlsx', marks=td.skip_if_no('openpyxl')),
+    pytest.param('openpyxl', '.xlsm', marks=td.skip_if_no('openpyxl')),
+    pytest.param('xlwt', '.xls', marks=td.skip_if_no('xlwt')),
+    pytest.param('xlsxwriter', '.xlsx', marks=td.skip_if_no('xlsxwriter'))
 ])
 class TestExcelWriter(_WriterBase):
     # Base class for test cases to run with different Excel writers.
@@ -1198,12 +1194,10 @@ def test_raise_when_saving_timezones(self, engine, ext, dtype,
 class TestExcelWriterEngineTests:
 
     @pytest.mark.parametrize('klass,ext', [
-        pytest.param(_XlsxWriter, '.xlsx', marks=pytest.mark.skipif(
-            not td.safe_import('xlsxwriter'), reason='No xlsxwriter')),
-        pytest.param(_OpenpyxlWriter, '.xlsx', marks=pytest.mark.skipif(
-            not td.safe_import('openpyxl'), reason='No openpyxl')),
-        pytest.param(_XlwtWriter, '.xls', marks=pytest.mark.skipif(
-            not td.safe_import('xlwt'), reason='No xlwt'))
+        pytest.param(_XlsxWriter, '.xlsx', marks=td.skip_if_no('xlsxwriter')),
+        pytest.param(
+            _OpenpyxlWriter, '.xlsx', marks=td.skip_if_no('openpyxl')),
+        pytest.param(_XlwtWriter, '.xls', marks=td.skip_if_no('xlwt'))
     ])
     def test_ExcelWriter_dispatch(self, klass, ext):
         with ensure_clean(ext) as path: