BUG: read_csv does not close file during an error in _make_reader

[twoertwein]

• closes BUG: read_csv - file left open after UnicodeDecodeError when sep=None #39024
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

I don't understand why td.check_file_leaks doesn't complain about the left opened file (at least for me locally). I commented the close call on purpose to see whether the test fails at least for the CI.

@jbrockmendel I think you have debugged ResourceWarnings in the past. Do you know why the test doesn't fail? Even putting a open("foo", mode="w") in the test doesn't make it fail.

[The test case is different from #39024 but the symptoms are the same. Unless the except clause is narrowed down to specific exceptions, this PR will fix #39024]

[pep8speaks]

Hello @twoertwein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-13 15:25:47 UTC

[jbrockmendel]

are the problems only on windows? one of the exception messages suggested that the file was being held by another process, maybe something is being spawned in windows that isnt spawned in non-windows?

come to think of it, when the tests are run with multiple processes, i dont think check_file_leaks would count files opened by sub-processes.

[twoertwein]

the original issue only happens on windows:

1. exception caused by some encoding but read_csv doesn't catch the exception
2. read_csv fails to close its file handles because it didn't catch the exception
3. os.remove (on windwos) can't remove the file because the file is still opened for reading

This PR uses a different exception for (1) and instead of (2) my idea was to simply check for opened files (should also fail on linux).

[twoertwein]

@jreback ping

[jreback]

thanks @twoertwein

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

$ git checkout 1.2.x
$ git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 4a3fa6973bee841978ff94cd4cba03425d703787

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #39029: BUG: read_csv does not close file during an error in _make_reader'

4. Push to a named branch :

git push YOURFORK 1.2.x:auto-backport-of-pr-39029-on-1.2.x

5. Create a PR against branch 1.2.x, I would have named this PR:

"Backport PR #39029 on branch 1.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

[jreback]

@meeseeksdev backport 1.2.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

$ git checkout 1.2.x
$ git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 4a3fa6973bee841978ff94cd4cba03425d703787

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #39029: BUG: read_csv does not close file during an error in _make_reader'

4. Push to a named branch :

git push YOURFORK 1.2.x:auto-backport-of-pr-39029-on-1.2.x

5. Create a PR against branch 1.2.x, I would have named this PR:

"Backport PR #39029 on branch 1.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

[jreback]

cc @twoertwein if you wouldn't mind putting up a backport PR for this