Skip to content

Conversation

@mgeier
Copy link
Contributor

@mgeier mgeier commented Oct 1, 2015

We had some discussions in the past about argument ordering (#54, #78), but I don't think we really talked about the order of the filename and the array argument in soundfile.write().

I always thought it would make sense to write the array first and the filename later.
I would ask "What?" first, before I want to know "Where?".
It also makes some kind of sense if you think about the array as an "input" argument and the filename as an "output" argument. I'd normally mention the input arguments before the output arguments.

However, while I'm using the function, I come to think more and more that the order actually seems strange.
What bugged me in the beginning, is that the array and its sample rate (which belong together), are actually separated in the call (and the sample rate is obligatory!):

sf.write(data, 'my_audio_file.wav', fs)

Then I looked around and found that it's actually quite common to mention the file name first, e.g.:
http://docs.scipy.org/doc/numpy/reference/generated/numpy.savetxt.html
http://docs.scipy.org/doc/numpy/reference/generated/numpy.save.html
http://docs.scipy.org/doc/scipy/reference/generated/scipy.io.wavfile.write.html#scipy.io.wavfile.write
http://docs.scipy.org/doc/scipy/reference/generated/scipy.io.savemat.html
http://docs.scipy.org/doc/scipy/reference/generated/scipy.io.mmwrite.html#scipy.io.mmwrite

I hereby suggest that we change the order to:

sf.write('my_audio_file.wav', data, fs)

Apart from it being the way NumPy and SciPy do it, it would also be more consistent to (and compatible to) soundfile.read(), where the array and the sample rate are returned right next to each other.

Are there any counter-examples that mention the data to be saved before the file name?
Are there any other arguments?

@bastibe
Copy link
Owner

bastibe commented Apr 29, 2015

I noticed that, too, and agree with your assessment.

@mgeier mgeier added this to the 0.8.0 milestone May 5, 2015
@mgeier
Copy link
Contributor Author

mgeier commented May 5, 2015

OK, then I suggest that we release 0.7.1 first (and probably 0.7.2?) and implement the current issue afterwards, for realease of 0.8.0.

@mgeier mgeier changed the title Change order of arguments in soundfile.write()? Change order of arguments in soundfile.write() May 5, 2015
@mgeier
Copy link
Contributor Author

mgeier commented May 5, 2015

Alternatively, we can just skip 0.7.1 and go directly to 0.8.0 ...?

@bastibe
Copy link
Owner

bastibe commented May 18, 2015

We have a bunch of changes again. Let's skip 0.7.1 and go straight to 0.8.0

@mgeier
Copy link
Contributor Author

mgeier commented May 21, 2015

In the past, I came up with breaking changes right after a release (e.g. #102) ...

I think it would be good to release 0.7.1 very soon, then we should think very hard if there is anything that can be improved by a breaking change, and afterwards release 0.8.0 with all breaking changes.
If we're lucky, this might become the last release with breaking changes in a long time (but I wouldn't guarantee it ...).

@bastibe
Copy link
Owner

bastibe commented Sep 30, 2015

Let's do this!

@mgeier
Copy link
Contributor Author

mgeier commented Oct 1, 2015

I've added a commit (8838390) that switches the two arguments.

@bastibe
Copy link
Owner

bastibe commented Oct 1, 2015

Looks innocuous enough.

@mgeier mgeier merged commit 8838390 into master Oct 1, 2015
@mgeier mgeier deleted the issue-132 branch October 1, 2015 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants