Handle 'rw' files correctly in __init__()

[mgeier]

The problem is that 'rw' files can be two quite different things: either existing files or new files.
If it's a new file, some arguments are required (like channels, subtype etc.), if it's an already existing file, those arguments are forbidden (except for 'RAW' files).

As far as I know, there is no function in libsndfile that allows to check before opening which of the two cases we're dealing with.

I see three options which could theoretically work:

1. Use a function from the Python standard library to check if the file already exists
2. Try to open the file with libsndfile. If there is an error, check if the reason are the required arguments. If there is no error, check if any of the forbidden arguments were given and raise an error.
3. Split 'rw' mode into two distinct modes, one that only works for existing files and another one that only works for new files (or alternatively, it can also delete the existing file content first, like 'w+' mode does in Python's open() function)

Are there more options?

I'm currently leaning towards option 3, see also #60.

[mgeier]

As an alternative to #60, I just created #65, which partly implements option 2 from above.
It doesn't check for forbidden arguments, though (they are just silently ignored).

Both have their shortcomings, but I still slightly prefer option 3 (with its implementation in #60).

[bastibe]

So, to summarize, our options are:

1. Keep everything the way it is. This conflates 'r+' with 'x+', depending on whether the file exists or not. This could theoretically be disambiguated with a manual check, but at any rate, this leaves the nomenclature in an unpleasant place that is neither pythonic nor sndfile-like.
2. Go with straight sndfile nomenclature (Change argument checking in SoundFile.__init__() #65). The main disadvantage is that 'rw' conflates 'r+' and 'x+' in an occasionally unpleasant way. On the other hand, the implementation is somewhat simpler.
3. Try to be as pythonic as possible (Change open modes to be more in line with Python's open() function #60). This will probably open every file in virtual-IO mode and the implementation will be a bit more complicated. The upside is that we can support most of the open modes of open. A related simplification would unify the read- and write-pointer (though this might prove problematic for non-seekable file types).

I see this mainly as a trade-off between interface pythonicity and implementation simplicity.

Personally, it took me quite a while to wrap my head around the intricacies of the separated read/write pointers and the different truncation behavior of libsndfile's modes (as apparent from our discussions here). For this reason, I am slightly in favor of 3. PySoundFile is better for being less surprising and more pythonic. However, we should be vigilant about the implementation complexity--if 3 proves to have too many unforeseen consequences and require too many unforeseen workarounds, we should probably back down and go with 2.

Is there anything important I missed? What do you think?

[mgeier]

Is there anything important I missed?

No, that's about it.
I just want to add that the complexity is currently not that much worse in #60 than in #65.
OTOH, the error handling is much cleaner in #60.

We'll have to deal with the non-seekable problem anyway, #67 has to be fixed no matter what we decide here.
The problem may get worse though, because in #60 additional seek() calls are introduced.
And the solution will probably add more complexity to #60.

I guess we should first check how we can solve the non-seekable problem in #60 and then we can decide if #60 or #65 (or something else) is the best choice.

[mgeier]

I added checks for the seek() calls in #60 (046023c).
It was only a minor thing, the complexity wasn't increased significantly.

I still prefer #60 over #65, because the slightly larger complexity buys much clearer semantics and much better error handling.
I think it's also much better than the current state.

I vote for merging #60!

What do you think?

[bastibe]

I agree!

[mgeier]

I merged #60 and closed #65, therefore I'm also closing this issue.