Move ExcelWriter Tests #26740
Move ExcelWriter Tests #26740
Conversation
WillAyd
added
IO Excel
|
|
||
|
|
||
| @pytest.fixture(params=['.xls', '.xlsx', '.xlsm']) | ||
| def read_ext(request): |
There was a problem hiding this comment.
The majority of this PR is a result of renaming this fixture so as not to conflict with the valid extensions for writing
|
|
||
| assert f.closed | ||
|
|
||
|
|
||
| @td.skip_if_no('xlrd') | ||
| @pytest.mark.parametrize("ext", ['.xls', '.xlsx', '.xlsm']) |
There was a problem hiding this comment.
This parametrization is still dangling and probably should use read_ext but for now I'm putting any tests that concern themselves with writing on the back burner, since reading is where most community engagement has been outstanding
Codecov Report
@@ Coverage Diff @@
## master #26740 +/- ##
==========================================
- Coverage 91.78% 91.77% -0.01%
==========================================
Files 174 174
Lines 50703 50703
==========================================
- Hits 46538 46534 -4
- Misses 4165 4169 +4
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #26740 +/- ##
==========================================
- Coverage 91.78% 91.77% -0.01%
==========================================
Files 174 174
Lines 50703 50703
==========================================
- Hits 46538 46534 -4
- Misses 4165 4169 +4
Continue to review full report at Codecov.
|
|
so maybe as a pre-cursor (after ok too) I would create pandas/tests/io/excel/test_excel, test_excel_writer and so on to make this much easier to grok |
|
Yea definitely needs to be split into submodules. I can do that one next PR |
|
ok thanks @WillAyd @simonjayhawkins quick look if you would. merge if ok. |
There was a problem hiding this comment.
@WillAyd lgtm. it might be worth (in a follow-on), since read_excel is patched anyway, to patch for the filename extension as well. it could remove some visual noise.
|
thanks @WillAyd |
Because read_excel and ExcelWriter have different parametrization requirements it makes things clearer for now to separate these out.
Maybe longer term they can be more intelligently combined but for now I think this still helps clean up requirements for adding new engines
@simonjayhawkins