Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing store_data_start in recursive peak splitter calls and peaklet["length'] fix in store_downsampled_waveform #920

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

HenningSE
Copy link
Collaborator

What is the problem / what does the code in this PR do

This PR fixes some smaller problems related to the data_start handling in the peaklets building/ splitting.

  • The flag store_data_start was not passed to recursive calls of the peak splitter. As a result some peaklets had data_start always set to 0.
  • peaklet["length'] is used in store_downsampled_waveform to select the part of the waveform that is stored in data_start. If the waveform is downsampled, peaklet["length'] is overwritten. When saving data_start we need to take the initial length of the peaklet not the downsampled length.

@coveralls
Copy link

coveralls commented Nov 5, 2024

Coverage Status

coverage: 90.161% (+0.002%) from 90.159%
when pulling d5b02d7 on bugfixing_data_start
into 0e7f9c6 on master.

@HenningSE HenningSE marked this pull request as ready for review November 5, 2024 15:00
@HenningSE HenningSE requested a review from dachengx November 5, 2024 15:00
@dachengx
Copy link
Collaborator

dachengx commented Nov 6, 2024

@HenningSE Please make the title of this PR more detailed.

@HenningSE HenningSE changed the title Bugfixing for data_start Fixing store_data_start in recursive peak splitter calls and peaklet["length'] fix in store_downsampled_waveform Nov 6, 2024
@dachengx dachengx merged commit b83f169 into master Nov 8, 2024
8 checks passed
@dachengx dachengx deleted the bugfixing_data_start branch November 8, 2024 13:01
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