Fixed read_seq for reading v1.3.1 sequence files

[FrankZijlstra]

• Removed some references the to lengths field in event_lib. I removed this field in previous PRs because it was completely unused, but missed these uses.
• Fixed the handling of block durations and delays for 1.3.1 sequence files in read_seq.

Should fix #161

[schuenke]

thanks for the fix @FrankZijlstra

I reviewed the code and tested it with several old sequences and it seems to work as expected.

However, IMO it should also work without the lines 257-260, no ?! We override this temp value anyway and it shouldn't matter if we overwrite a temporary delay value or just 0, does it?

In general, the types in event_lib are not all correct and the employed update() method doesn't require old data, which you just consequently set to None here , but this is something we can discuss during the code refactoring process and not here I guess.

[schuenke]

is this needed?

[FrankZijlstra]

Yes, it is needed. In the old format, delay events were explicitly defined in the event table. In the new format, only the block duration is stored in the event table. However, to calculate the duration, we need the maximum of the stored delay and the actual duration of the block events (determined with calc_duration). get_block gives us the stored block duration (where you previously got an error), i.e. the delay, which is used by calc_duration (but the output of calc_duration might be larger depending on the events).

I do feel the whole handling of the delay event is a bit messy in the new format. We define a delay event with make_delay, and can use it in calc_duration. But as soon as we add it to the sequence, the delay event disappears. Instead, get_block will just give us a block_duration field, which is a special case in calc_duration (isinstance(event, (float, int))). As a consequence, the only reason calc_duration can determine the duration in this case, is because the duration was already stored.

[schuenke]

Ah, thanks for the nice explanation. Wasn't aware that the delay event is gone after adding it to a block.

[FrankZijlstra]

In general, the types in event_lib are not all correct and the employed update() method doesn't require old data, which you just consequently set to None here , but this is something we can discuss during the code refactoring process and not here I guess.

When I removed the use of old_data before, I did consider just removing it altogether, but figured that it would be a good idea to keep compatibility (in the unlikely case someone was ever adventurous enough to mess with the event libraries...). But I think it's pretty safe to remove altogether.