REGR: SpooledTemporaryFile support in read_csv

[twoertwein]

• closes BUG: SpooledTemporaryFile no longer working with read_csv #43439
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

Probably should throw a warning, if we cannot wrap the handle with io.TextIOWrapper.

To avoid the try-except, we could check whether the handle has the readable attribute, but there might be other attribute errors that might occur inside TextIOWrapper.

[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-09-09 21:59:11 UTC

[simonjayhawkins]

Thanks @twoertwein. Is this targeting 1.3.x?

[twoertwein]

Thanks @twoertwein. Is this targeting 1.3.x?

It should be, I'll probably have time this evening to finish it up.

I'm not yet sure whether try-except is a good solution here. Another option (maybe more for 1.4), would be to add a new keyword to read_csv to set the mode (to_csv has that currently). Then a user could overwrite the binary/text auto-detection by specifying mode="rt" or mode="rb".

[twoertwein]

Instead of using a try-except block, I added tempfile.SpooledTemporaryFile to the list of classes that should always be treated as text handles.

I'm not sure whether #43439 is a regression: yes, it worked previously, but it only worked because of inconsistencies between the c-engine and python-engine. This PR allows this inconsistency to continue (I'm not a fan of this). It is great that the c-engine is not only faster than the python-engine, but also doesn't seem to need theio.TextIOWrapper (skipping the wrapper for the c-engine might actually give a slight performance improvement).

I would be slightly in favor of not fixing #43439 for 1.3.3 and instead adding a new keyword argument ("mode", same as in to_csv) in 1.4 for read_csv which allows the user to over-write the text/binary auto-detection. (And a PR for 1.4 to make the c-engine "dumber": if it is told to get strings, but it gets bytes, it should fail.).

If it is preferred to fix #43439 for 1.3.3, I'm happy to make this happen (I think this PR should be ready).

[jreback]

oh new commit way nicer

[jreback]

very nice @twoertwein

[jreback]

@meeseeksdev backport 1.3.x

[lumberbot-app]

Something went wrong ... Please have a look at my logs.