Skip to content

analysis.split sorts change points before getting intervals - #117

Merged
henrikingo merged 2 commits into
masterfrom
fix-perf-test
Dec 21, 2025
Merged

henrikingo merged 2 commits into
masterfrom
fix-perf-test

Conversation

@Gerrrr

@Gerrrr Gerrrr commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

Prior to this patch, change points discovered while scanning sliding
windows are added to the end of the list. At the same time,
SignificanceTester.get_intervals expects change points to be sorted.

As a result, intervals contain 1 or more None that cause ValueError in
tester.change_point.

The second commit adds perf tests to the CI suite, so we catch this kind of bugs during PR review.

Prior to this patch, change points discovered while scanning sliding
windows are added to the end of the list. At the same time,
SignificanceTester.get_intervals expects change points to be sorted.

As a result, intervals contain 1 or more None that cause ValueError in
tester.change_point.
@Gerrrr
Gerrrr marked this pull request as ready for review December 18, 2025 07:38
@Sowiks

Sowiks commented Dec 19, 2025

Copy link
Copy Markdown
Contributor

Went over the code changes, it's an approval from me.
Good catch!

@henrikingo

Copy link
Copy Markdown
Contributor

Now the question is, should we track our own perf test results with a dashboard and change detection?

@henrikingo
henrikingo merged commit 7ba4186 into master Dec 21, 2025
6 checks passed
@Gerrrr

Gerrrr commented Dec 22, 2025

Copy link
Copy Markdown
Contributor Author

should we track our own perf test results with a dashboard and change detection?

This would be lovely! Can we use Nyrkiö dashboard? Maybe we can start a mailing thread about it.

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