TST/CLN: declass smaller test files in tests\io\excel #26764
Conversation
simonjayhawkins
added
Clean
Testing
Codecov Report
@@ Coverage Diff @@
## master #26764 +/- ##
==========================================
- Coverage 91.71% 91.7% -0.01%
==========================================
Files 178 178
Lines 50740 50740
==========================================
- Hits 46538 46533 -5
- Misses 4202 4207 +5
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #26764 +/- ##
==========================================
- Coverage 91.71% 91.7% -0.01%
==========================================
Files 178 178
Lines 50740 50740
==========================================
- Hits 46538 46533 -5
- Misses 4202 4207 +5
Continue to review full report at Codecov.
|
|
|
||
|
|
||
| def test_to_excel_styleconverter(ext): | ||
| from openpyxl import styles |
| "hidden": False, | ||
| openpyxl = pytest.importorskip("openpyxl") | ||
|
|
||
| pytestmark = pytest.mark.parametrize("ext", ['.xlsx']) |
There was a problem hiding this comment.
isn't this just a fixture definition?
There was a problem hiding this comment.
do we want more fixtures?
we could have a fixture and inject into all tests or we could alias this mark and decorate all tests.
this just applies the parametrisation to all tests in module automatically.
There was a problem hiding this comment.
Not yet but we should probably create a write_ext fixture for use here and in the writer tests (the existing fixture is applicable to extensions that we read)
There was a problem hiding this comment.
i just find this very opaque here (we don't use this idiom anywhere else)
There was a problem hiding this comment.
Probably better done in a separate PR IMO as that will require a decent amount of updates
|
|
||
|
|
||
| def test_write_cells_merge_styled(ext): | ||
| from pandas.io.formats.excel import ExcelCell |
There was a problem hiding this comment.
can you move imports to the top
| read_workbook = openpyxl.load_workbook(path) | ||
| try: | ||
| read_worksheet = read_workbook['Sheet1'] | ||
| except TypeError: |
There was a problem hiding this comment.
OT: are these still needed? (the excepts)
|
|
||
| import pandas as pd | ||
| import pandas.util.testing as tm | ||
| from pandas.util.testing import ensure_clean | ||
|
|
||
| from pandas.io.excel import ExcelFile | ||
|
|
||
| xlrd = pytest.importorskip("xlrd") | ||
| xlwt = pytest.importorskip("xlwt") |
There was a problem hiding this comment.
Do we already have a file we can use that contains the content of frame? Having to include xlwt creates an unnecessary dependency in this module
| "hidden": False, | ||
| openpyxl = pytest.importorskip("openpyxl") | ||
|
|
||
| pytestmark = pytest.mark.parametrize("ext", ['.xlsx']) |
There was a problem hiding this comment.
Not yet but we should probably create a write_ext fixture for use here and in the writer tests (the existing fixture is applicable to extensions that we read)
| "hidden": False, | ||
| openpyxl = pytest.importorskip("openpyxl") | ||
|
|
||
| pytestmark = pytest.mark.parametrize("ext", ['.xlsx']) |
There was a problem hiding this comment.
Probably better done in a separate PR IMO as that will require a decent amount of updates
|
@simonjayhawkins these comments can be done as a followup (or make an issue) if you prefer |
OK. AFK for a while now anyway. |
|
thanks @simonjayhawkins |
i think i'll open an issue as well, as there are comments from this and several of the recent test_excel cleanup PRs to be considered as follow-ups and it'll be easier to track. |
xref #26755 (review)
the larger files
pandas/tests/io/excel/test_readers.pyandpandas/tests/io/excel/test_writers.pyhave been left for separate PRsany idiom changes other than required to declass are also omitted to make the diff easier to check.