Remove file handle support and type check in save and load

[craffel]

Currently, if a unicode string was passed to deepdish.io.load or save, it would fail because isinstance(u'file.h5', str) = False. This PR allows for all six.string_types to be passed.

More generally, I'd suggest something like this, assuming it covers your intended use-cases:

if isinstance(path, file):
    path = path.name
elif not isinstance(path, six.string_types):
    raise ValueError('path type {} not supported.'.format(type(path))

[gustavla]

Yes, I absolutely agree. I do like your suggested change of isinstance(path, file) the most, so feel free to update the PR with that instead (or I'll just do it after the PR has been merged).

Thanks for the contribution!

[craffel]

Ok, updated.

[gustavla]

Tests failed. There is no file in Python 3, so checking whether or not an object is file-like is not entirely clear how it should be done correctly. This is coming back to me now. I think I initially tried to do this as well.

I'm thinking your first suggestion might be the best. There is hasattr(path, 'name'), but that seems like a bad idea.

[gustavla]

There are a few more suggestions here: http://stackoverflow.com/questions/1661262/check-if-object-is-file-like-in-python

[craffel]

Ah, sure, ok. Looking at the code what I'd actually suggest is the same paradigm that numpy uses: https://github.com/numpy/numpy/blob/master/numpy/lib/npyio.py#L473 Basically, if isinstance(path, six.string_types) = True you open the file for reading or writing as fid and set a flag own_fid to True, then if own_fid is True you fid.close() at the end of the function. If isinstance(path, six.string_types) = False you treat it like a file pointer, and don't open or close anything. This is instead of using path.name and the with open context. If you'd like, I can implement this. I did something similar here: https://github.com/craffel/mir_eval/blob/master/mir_eval/io.py#L51

[gustavla]

Actually, I'm starting to think it's better to throw out the functionality to allow a file handler altogether, because it is clearly not supporting it in any other way than fetching its .name. if anyone tries to make a custom file handler that saves directly to an exotic data storage, it won't work (even though you get the impression that it would). PyTables only allows opening files through path strings, so that is the only thing we can truly support. Although, we could support if a PyTables file handler is passed in.

My suggestion is to use your initial proposal, but add a deprecation warning if it receives a file handler. Then, it can be removed over time. What do you think?

[craffel]

PyTables only allows opening files through path strings, so that is the only thing we can truly support.

Ah, I didn't realize that.

My suggestion is to use your initial proposal, but add a deprecation warning if it receives a file handler. Then, it can be removed over time. What do you think?

Deprecation vs removal is up to you, depending on how many people are using deepdish and if any of them are actually using the file handler pattern. Github search suggests no known uses: https://github.com/search?q="deepdish"+"io.load"&ref=searchresults&type=Code&utf8=✓

[gustavla]

Yeah, we can probably just remove it altogether right away without no one ever noticing. So, the final solution is to have no if statement altogether. There is a unit test in test_io.py that will need to be removed for the tests to pass.

[craffel]

Ok, updated.

[gustavla]

I see you just noticed the doc update too, terrific!

Thanks so much for this! I'm really happy with this change.

[craffel]

Happy to help, I'm excited about deepdish.