Avoid invalid files by error-prone mode flags of open

[tbeu]

From #37 and #38 we learned that opening the recon file requires mode='rb' for reading and mode='wb+' for writing since it turned out that not setting the binary 'b' mode may lead to invalid recon files.

with open('dsres', 'rb') as wfp:
    with open('mld', 'wb+') as mfp:
        dsres2meld(wfp, mfp)

This is error-prone since developers might forget to set the binary 'b' mode. For that reason I propose to introduce a new file wrapper class, say recon.reconFile. Mode flags then could be similar to zipfile.ZipFile with valid settings like mode='r' or mode='w'. Finally all functions that currently take file handles (i.e. isinstance(wfp, file) yields True) shall check their arguments for type recon.reconFile (i.e. isinstance(wfp, recon.reconFile) must yield True).

with recon.reconFile('dsres', 'r') as wfp:
    with recon.reconFile('mld', 'w') as mfp:
        dsres2meld(wfp, mfp)

def dsres2meld(wfp, mfp):
    if isinstance(wfp, recon.reconFile) and isinstance(mfp, recon.reconFile):
        print('OK: These are the expected file type, go ahead')

[xogeny]

Have a look at these changes. They do not introduce a reconFile type, but I think they address the issue.

https://github.com/xogeny/recon/compare/issue45

The bottom line is that this allows either a file pointer or a file name to be passed into the Reader and Writer classes. In the case of a file name, the file pointer is created internally.

One nice thing about implementation on branch issue45 is that it fully supports the with construct in Python. In other words, you can do this:

  with MeldReader("foo.mld") as meld:
    print "Tables: "+str(meld.tables())

...and it will automatically close the file foo.mld when then with context is closed.

Anyone see any issues with this?

[tbeu]

Good solution to ensure a backward-compatible api of MeldReader and MeldWriter. Tiny safety-check: In case it is not a file name I would not assume it is a file but rather check if it is really of type file.

[xogeny]

I thought about this. But the question is...what makes it a file? type(...)==file is one way, but I'm afraid that will be too limiting (you can see a discussion of the issues here).

Note that if you went the hasattr route, you'd need to check not only for read and write but also for seek and tell which might be considered more specialized than just file (I'm not sure).

[tbeu]

Hm, I was thinking of isinstance(something, file) but if this is too limiting I can live without it.

[xogeny]

OK. I think we at least have a solution to the original issue (of avoiding non-binary opening of files). Maybe we can make it better in the future, but I'm considering this closed as of 43960e15a35ae8b4af13eb0248614130dbcfc29e.