Skip to content

Add append(), overwrite() and accumulate()?#86

Closed
bastibe wants to merge 1 commit intomasterfrom
new-functions
Closed

Add append(), overwrite() and accumulate()?#86
bastibe wants to merge 1 commit intomasterfrom
new-functions

Conversation

@bastibe
Copy link
Owner

@bastibe bastibe commented Mar 11, 2015

I'm glad that __getitem__ and __setitem__ will be deprecated (see #75). I think that most of their functionality can be easily done with the other available functions and methods (and often in a better way).

There is only one set of functions where indexing could be quicker (but not necessarily better) than the currently available options:

  • append(): open a file in 'r+' mode, seek to the end, write a given array of data, close the file.
  • overwrite(): open a file in 'r+' mode, seek to a given position, write a given array of data, close the file.
  • accumulate(): open a file in 'r+' mode, seek to a given position, accumulate a given array of data to the data which is already in the file, close the file.

Note that all of these only really make sense in 'r+' mode.
Note also, that append() is only a special case of overwrite().
accumulate() is probably too specialized to deserve its own function and it's also my least favorite of the 3.

I personally cannot imagine that I would ever need any of these, but if someone else needs them, I'm fine with supporting them.

@bastibe
Copy link
Owner

bastibe commented Nov 17, 2014

Very interesting idea! This would certainly solve many use cases very elegantly. I like it!

Let's do it after 0.6 though.

@bastibe bastibe added this to the future milestone Nov 17, 2014
@bastibe bastibe self-assigned this Mar 4, 2015
@bastibe bastibe modified the milestone: future Mar 4, 2015
@faroit
Copy link

faroit commented Mar 11, 2015

👍

@bastibe
Copy link
Owner

bastibe commented Mar 11, 2015

It turns out that you can write append as a special case of overwrite, but this requires opening the file twice (once for getting its length, and once for overwriting). I opted for not making it a special case instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think channels has to be removed here.
It should be obtained from data, like in the write() function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for overwrite() and accumulate(), of course.

@mgeier
Copy link
Contributor Author

mgeier commented Mar 18, 2015

It turns out that you can write append as a special case of overwrite, but this requires opening the file twice (once for getting its length, and once for overwriting).

I think this is just a matter of the API of overwrite(). To be able to re-use it, it's necessary to expose the whence argument of seek().

I think those three "situations" are way too specialized (and I suspect too seldom used) to deserve three distinct top-level functions.

If we expose whence and add a accumulate argument (with default value False), we can get everything in one function:

  • overwrite: overwrite(mydata, 'myfile.wav', start=27)
  • append: overwrite(mydata, 'myfile.wav', start=0, whence=sf.SEEK_END)
  • accumulate: overwrite(mydata, 'myfile.wav', start=44, accumulate=True)

There is still an inconsistency with the argument start, which I guess is motivated by the identically named argument in the read() function, but it (naturally) has no according frames and stop arguments.
OTOH, in truth, this is the argument frames from the seek() method, so there is some potential for confusion there.

Apart from all that, I'm still not convinced that any of this is necessary at all.

I think such specialized convenience functions are only justified if they are used often and if they are used in interactive sessions.
In "normal" Python code it's really easy to write a with statement and two to four more lines, and it may be even easier to understand and most likely easier to maintain than using one of the proposed functions!

@faroit: It's really helpful to get feedback, thanks for that!
But can you please elaborate a bit about how you would use those functions and share your opinion about if they are necessary or not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the repetition of those details. I would just refer to the function :func:write``.

@bastibe
Copy link
Owner

bastibe commented Sep 30, 2015

We agreed to not do this.

@bastibe bastibe closed this Sep 30, 2015
@bastibe bastibe deleted the new-functions branch April 25, 2016 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants