ENH: Expanduser in to_file methods GH9066

[hsperr]

closes #9066

First time contributor, tried to follow all rules as good as possible.
If something is missing please let me know.

As specified in task 9066 tilde in a pathname get resolved into a absolute path.
In case the object is a buffer the buffer will be returned.

I did not add additional tests, since I could not find any module explicitly testing pandas/io/common.py
if that is necessary please let me know how to do that. The current tests are passing on my system and seem to cover the method below.

Best Regards,

[jreback]

@jorisvandenbossche worth adding a tests_common.py in pandas/io ?

[shoyer]

Is there a specific exception you are trying to catch here?

[hsperr]

while testing the feature it looked like filepath_or_buffer can be a StringIO object which throws an exception in os.path.expanduser(), then I thought I'd stick to the convention like _is_url() or _is_s3_url() where they try to parse it as one and treat it otherwise on except.

[shoyer]

Ah... I know we have some bad examples in the existing codebase, but please don't add new blanket except clauses. (Under _is_s3_url, it really should be except ImportError.)

If expanduser does not raise a sane exception, you could use isinstance(filepath_or_buffer, pandas.compat.string_types)

[hsperr]

Hey thank you for the note, I will fix that and use the isinstance version.

[jorisvandenbossche]

I don't know if this is easy to test (as the home dir depends on where the tests are run, of course can reuse expanduser to just test that the feature is there), but if so, certainly a good idea to add a io/tests/test_common.py

[hsperr]

I now use isinstance instead of try/except to see whether the path might be expandable. I also tried rebasing the new version so it will only be two commits again but maybe my git knowledge is not good enough to do so, if that is necessary please let me know and I will read up/figure it out.

about testing, not sure if its good practice to test against builtins but we can test whether get_filepath_or_buffer() returns the same as os.path.expanduser given a string input and returns the StringIO given a non string input/url.

I will add some tests for that, thanks.

[jorisvandenbossche]

That is what I meant with "I don't know how easy it is to test", as it is indeed not very useful to test builtin behaviour. But what we can test, it just our API, the fact that the feature is there and will not accidentally be removed later on. So in that case, I think it is not bad to just reuse expanduser, or maybe just assure that something is added to the string)

[shoyer]

I might say something more explicit, e.g., "Paths beginning with ~ will now be expanded to begin with the user's home directory"

[shoyer]

@hsperr This looks great! Could you please squash this into a single change? See this page if you're not quite sure what that means.

[hsperr]

@shoyer Thanks a lot for the link to the rebase page, I indeed did not feel super comfortable with that but I think I managed to squash/fixup everything into a single commit. I also took your version for the changelog, thanks! If there is anything more let me know and hope I can contribute more in the future!

[shoyer]

Thanks @hsperr !