`blocks()` that also returns seek point/location of the block

[endolith]

I want something with all the same options as blocks() except that it also tells you where you are in the file, so you don't have to figure it out yourself, since that can be complicated with start/stop/blocksize/overlap.

So instead of

>>> n = start
>>> for block in sf.blocks('stereo_file.wav', blocksize=1024):
>>>     pass  # do something with 'block'
>>>     n += blocksize - overlap

it would be something like

>>> for n, block in sf.enumerate_blocks('stereo_file.wav', blocksize=1024):
>>>     pass  # do something with 'block' and 'n'

[mgeier]

Is it really that complicated?

start + i * (blocksize - overlap)

What about adding something like this as an example to the documentation instead?

filename = 'myfile.wav'
start = 100000
blocksize = 1000
overlap = 100
blocks = sf.blocks(filename, start=start, blocksize=blocksize, overlap=overlap)
for i, block in enumerate(blocks):
    block_start_index = start + i * (blocksize - overlap)
    # ...

It's also more flexible this way. People might not be interested in the start offset. If we provide the index for them, they'll have to subtract start on their own, which is about as complicated as computing the index in the first place.

[endolith]

Well it's several more lines of code that we have to remember or look up and hope we didn't make any off-by-one errors. It would be nice if it were done automatically for us without errors.

[bastibe]

I'm with @mgeier here. I think it is a great idea to add an example to the README, but if there is one thing I have learned with regards to library design, it is that a library should be minimal, and not include anything but the absolute necessary. It is easier for individual programmers to write customized wrappers than for a library to cover every possible use case.

[endolith]

Well if you take that argument far enough, blocks() and enumerate() shouldn't exist in the first place, since the same tasks can be accomplished without them :)

[bastibe]

If we implemented something like that, would we always start from zero? Does that even make sense in, say, a network stream? Another consideration is that this would definitely break backwards compatibility, which is not something we should do lightly.

In order to not break compatibility, maybe we could calculate the sample index as a method on the iterator, or only optionally return it if the generator is given a function parameter, or even assign the sample index as a new attribute on the block ndarray. What would you two think of those ideas?

[endolith]

If we implemented something like that, would we always start from zero?

The intent is to know where you are in a file while analyzing it, so it would always be the location in the file. If start is not zero, then it would not start at zero.

Does that even make sense in, say, a network stream?

I don't think so, no

Another consideration is that this would definitely break backwards compatibility, which is not something we should do lightly.

How would it break backwards compatibility? My proposal is a separate function, just like blocks(), except that it returns both pieces of information in each loop.

In order to not break compatibility, maybe we could calculate the sample index as a method on the iterator

How would this be used?

or only optionally return it if the generator is given a function parameter

That would be fine, too. Then if you thought there was a good reason to always start at 0, even if start was not zero, that could be another option. return_sample_value={None, 'file', 'relative'} or something. This seems like the way things are normally done in scipy, such as lfilter returning zi only if you call it with zi.

or even assign the sample index as a new attribute on the block ndarray.

So it would be like this?

for block in blocks():
    data = block
    position = block.sample_number

That would be ok, too, if creating custom attributes is a legitimate thing to do.

[mgeier]

@endolith It wasn't entirely clear from your original proposal if you wanted to replace or extend the current functionality. I'm glad this has been cleared up.

I don't agree with @bastibe's "minimal" approach and I'm very much a fan of convenience functions. And as you said, blocks() is pure convenience, since it could be implemented somewhat easily with the rest of the public API.

However, I think there should be a limit for adding new features, and this limit should be decided on a case-by-case basis.

As you showed yourself, your suggested feature would be very simple to implement in user code, a new function name or a new argument might actually put more cognitive burden on users.

It's also kind of hard and annoying to implement correctly inside the library. In order to be consistent, the method SoundFile.blocks() would also have to be changed. It would make sense there, to use tell() (or similar) to get the current position in the file. Sadly, this wouldn't work with non-seekable files. We would have to add checks for seekability all over the place and probably a few error messages. All in all, this doesn't sound like it is worth the hassle for such a minor (alleged) improvement.

Still, I'm not saying it's impossible.

we could calculate the sample index as a method on the iterator

I think this would be a very uncommon idiom in Python. It wouldn't work with a generator function (which could be reason enough not to do it), so we would have to re-implement the whole thing with a separate class using the iterator protocol.

or even assign the sample index as a new attribute on the block ndarray

This doesn't sound like a good idea. Especially if a custom ndarray subclass is given as out parameter. This subclass might e.g. disallow creating new attributes. Or we might overwrite an existing attribute. And again, this doesn't seem to be a common Python/NumPy idiom (or is it?).

only optionally return it if the generator is given a function parameter

I think that's the only reasonable possibility. return_sample_value would be the wrong name, it would rather be something like return_sample_index or more correctly return_frame_index. Having two different options would make sense, but the names 'file' and 'relative' are IMHO not very obvious without substantial documentation. I don't know any better names, either. And what to choose if start is 0? There isn't one obvious way to do that ... And most importantly: This will lead to less readable user code.

Long story short: it would make for a complicated implementation and for less readable user code.

for block_start_index, block in sf.blocks('myfile.wav', blocksize=1024, return_frame_index='file'):
    # ...

OTOH, everyone would understand this without hesitation:

blocksize = 1024
for i, block in enumerate(sf.blocks('myfile.wav', blocksize=blocksize)):
    block_start_index = i * blocksize
    # ...