Fix ring buffer gap cleanup code

[matthias-wende-frequenz]

When cleaning up gaps the case that a gaps start date can become older then the oldest stored value in the buffer wasn't covered. Furthermore the cases for cleaning up a single gap didn't run for the last gaps, since the loop was stopping to early.

[Marenz]

Def should have a test for this and the other new case

[matthias-wende-frequenz]

There is a case for the new case at least

[matthias-wende-frequenz]

Added another test case for the "old" bug.

[Marenz]

Your commit message claims

When cleaning up gaps the case that a gaps end date can become older
then the oldest stored value in the buffer wasn't covered.

But I don't think it's true. It's covered right here:

            # Delete out-of-date gaps
            if w_1.end <= self._datetime_oldest:
                del self._gaps[i]

[Marenz]

About having gaps whos start time is older than the oldest.. I am not sure that's really important, but I guess it doesn't hurt to clamp it

[Marenz]

Also, in your commit message you're wrongly using "then" when it should be "than" :)

[matthias-wende-frequenz]

Your commit message claims

When cleaning up gaps the case that a gaps end date can become older
then the oldest stored value in the buffer wasn't covered.

But I don't think it's true. It's covered right here:

            # Delete out-of-date gaps
            if w_1.end <= self._datetime_oldest:
                del self._gaps[i]

No, that case deletes the whole gap once it's fully outside of the buffer, i.e. it looks only at the end of the gap. The new case looks at and updates the beginning of the gap.

[matthias-wende-frequenz]

About having gaps whos start time is older than the oldest.. I am not sure that's really important, but I guess it doesn't hurt to clamp it

We need that in the moving window. But maybe it's not a bug, but more like we covering an unforeseen use case. We want to know the number of missing items in the window and now we can simply sum up the length of all gaps.

[Marenz]

I am sorry, but I am confused. The commit message talks literally about the end date being older than oldest timestamp

When cleaning up gaps the case that a gaps end date can become older
then the oldest stored value in the buffer wasn't covered.

And this matches also what you say my case is..

No, that case deletes the whole gap once it's fully outside of the buffer, i.e. it looks only at the end of the gap.

This though

The new case looks at and updates the beginning of the gap.

is true, however the message should then say "Clean up gap starts if they are before the oldest timestamp" or so

[Marenz]

PR LGTM, just the first commit message greatly confuses

[matthias-wende-frequenz]

I am sorry, but I am confused. The commit message talks literally about the end date being older than oldest timestamp

When cleaning up gaps the case that a gaps end date can become older
then the oldest stored value in the buffer wasn't covered.

And this matches also what you say my case is..

No, that case deletes the whole gap once it's fully outside of the buffer, i.e. it looks only at the end of the gap.

This though

The new case looks at and updates the beginning of the gap.

is true, however the message should then say "Clean up gap starts if they are before the oldest timestamp" or so

Did I ever mention that I'm having troubles with signs :D. Indeed the the start time of the gap is meant. I'll update the message accordingly.
To be fair. The code tells the truth :).

[llucax]

You are a bad bad boy. You should have first approved and merged #574.

[Marenz]

Wops