REGR: to_stata tried to remove file before closing it

[twoertwein]

Spin-off from #39047:

If an error occurred during to_stata, the code tried to first remove the file and then close the file handle:

1. removing the file failed on Windows (it is still opened)
2. It created an empty file when closing the handle

[jreback]

this is a regression (i think so?). put a note on 1.2.1

[twoertwein]

yes, you are right! I extended a test to make sure that the file doesn't exists and added a whatsnew entry.

[jreback]

excellent ping on greenish

[twoertwein]

The failing test will not fail in 'real life': the test uses tm.ensure_clean() to ensure that the file will not exist once the test is done. Unfortunately, this means that we have an open file handle that to_stata cannot close. I will look into re-writing ensure_clean().

[jreback]

also some failures

[twoertwein]

I think to_stata is the only to_* method that tries to remove created files in case of an error. Is that something other methods should also try to do?

I'm not sure but I could imagine that __exit__ in IOHandles might be able to determine whether an exception from within pandas's io part was raised: if yes, try to delete the files. This would probably only cover local files (no fsspec strings) and only work for to_* methods that use with get_handles(...) (some methods have to use it without the context manager and explicitly call close - IOHandles wouldn't know whether an exception occurred.

[jreback]

we should remove it if it's corrupt - however this might be hard to tell

so ok leaving it

[twoertwein]

@jreback green'ish

The new ensure_clean has the drawback (I see it as an advantage) that the code using it needs to make sure that no file handle references the temporary file. If an open file handle exists, the test on windows will fail. That's why I had to fix a few tests in this PR.

[jreback]

lgtm comment, ping on green

[twoertwein]

@jreback green'ish, failed tests seem to be unrelated

[simonjayhawkins]

failed test seem to be unrelated

yep, known flaky test and pre-commit issue

[simonjayhawkins]

one question before this is merged. How public-ish is tm.ensure_clean?

[jreback]

one question before this is merged. How public-ish is tm.ensure_clean?

its not

[jreback]

thanks @twoertwein

[jreback]

@meeseeksdev backport 1.2.x

[jreback]

@twoertwein would you mind putting up the backport PR