REGR: close corrupt files in ExcelFile #41806
Conversation
jreback
added
IO Excel
|
lgtm. ping when ready. |
| msg = "File is not a zip file" | ||
| with tm.ensure_clean(f"corrupt{read_ext}") as file: | ||
| with pytest.raises((BadZipFile, ValueError), match=msg): | ||
| with pd.ExcelFile(file, engine=engine) as _: | ||
| pass |
There was a problem hiding this comment.
this is testing the message from the pd.ExcelFile(file, engine=engine) as _ expression which does already raise BadZipFile: File is not a zip file
how is this testing that the file is closed?
There was a problem hiding this comment.
I can't replicate the issue locally to see this test fail without the patch.
I assume it's Windows specific (I use windows, but use wsl and don't have dev tools installed in native windows)
I think that ensure_clean is supposed to remove files, closed or not, so the file leaks check is checking ensure_clean and not the op under test?
There was a problem hiding this comment.
The test worked only on windows: tm.ensure_clean will fail to remove the file and throw an error (the file leak check doesn't catch the ResourceWarning).
Locally, the test fails for me now when the patch is not applied (but it might depend on whether ResourceWarnings are en/disabled).
|
@twoertwein some tests are failing, if you fix-up and remove the draft status so can be merged when green. (I think this is the last PR for 1.2.5, we have a few outstanding issues but no PRs to fix. https://github.com/pandas-dev/pandas/milestone/85 @jreback ) |
|
The remaining tests fail because I think we can avoid this bug by writing random data to the file. |
|
@simonjayhawkins @jreback green |
|
@meeseeksdev backport 1.2.x |
|
thanks @twoertwein |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
looks like needs manual backport |
@simonjayhawkins maybe for 1.2.5?