TST: Fix excel test discovery #16478
Conversation
TomAugspurger
added
IO Excel
|
oh, so these were not running? hope they pass! |
pandas/io/excel.py
Outdated
| @@ -202,6 +202,16 @@ def read_excel(io, sheet_name=0, header=0, skiprows=None, skip_footer=0, | |||
| dtype=None, true_values=None, false_values=None, engine=None, | |||
| squeeze=False, **kwds): | |||
|
|
|||
| # Can't use _deprecate_kwarg since sheetname=None has a special meaning | |||
| if sheet_name is 0 and 'sheetname' in kwds: | |||
There was a problem hiding this comment.
I think we want that here. sheet_name could be a str, int, or None and we don't want to raise a TypeError. Just checking whether they changed the default.
There was a problem hiding this comment.
Although, is that a CPython specific thing, pre-defining some integer objects? Maybe I should change it...
There was a problem hiding this comment.
Looks like it is specific to CPython. I'll update.
There was a problem hiding this comment.
Yeah, is only works at all because of the int cache. It's almost never what anyone wants.
Codecov Report
@@ Coverage Diff @@
## master #16478 +/- ##
==========================================
+ Coverage 90.42% 90.81% +0.39%
==========================================
Files 161 161
Lines 51029 51038 +9
==========================================
+ Hits 46144 46352 +208
+ Misses 4885 4686 -199
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16478 +/- ##
==========================================
+ Coverage 90.43% 90.82% +0.38%
==========================================
Files 161 161
Lines 51045 51038 -7
==========================================
+ Hits 46161 46353 +192
+ Misses 4884 4685 -199
Continue to review full report at Codecov.
|
TomAugspurger
force-pushed
the
excel-tests
branch
2 times, most recently
from
4a5fa36
to
f8ef9fa
Compare
Since sheetname=None has a special meaning, we can't use the deprecate_kwargs decorator. We instead handle it in read_excel.
Reader / writer may depend on filename and engine. Set these on the reader and writer before round-tripping.
TomAugspurger
force-pushed
the
excel-tests
branch
from
f8ef9fa
to
a3bcbd1
Compare
|
ahh this is fixing up after #16442? |
|
Yeah, 11bed21 fixes that. Would have been an unfortunate breakage :/ |
|
Ugh. That random matplotlib failure is spreading: https://ci.appveyor.com/project/pandas-dev/pandas/build/1.0.2225/job/6lg79jfhas0l9qiw#L1128 (I hadn't seen it on windows yet) @jreback ok to merge? Or can you restart appveyor? |
|
yep |
|
@TomAugspurger You will have to split this commit if you want to backport this PR, as the sheetname change is only targetted for 0.21.0 |
I hadn't thought of that... I think I'll not backport it then, unless there are objections. |
|
It's only about some excel test not running on the 0.20.x branch? That sounds OK to me to not backport if we don't have any excel related changes in 0.20.x |
* TST: Fix excel test discovery * BUG: Handle sheetname deprecation directly Since sheetname=None has a special meaning, we can't use the deprecate_kwargs decorator. We instead handle it in read_excel. * TST/BUG: Ensure pathlib roundtrip uses right params Reader / writer may depend on filename and engine. Set these on the reader and writer before round-tripping.
closes #16477
Locally: