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.

Sign up for GitHub

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

Jump to bottom

Remove configuration sum_waveform_top_array from MergedS2s #1415

Merged
merged 2 commits into from
Aug 27, 2024
Merged

Remove configuration sum_waveform_top_array from MergedS2s #1415

merged 2 commits into from
Aug 27, 2024

Conversation

dachengx
Copy link
Collaborator

@dachengx dachengx commented Aug 26, 2024 •
edited
Loading

Close: #1305

@coveralls
Copy link

coveralls commented Aug 26, 2024 •
edited
Loading

Coverage Status

coverage: 90.824% (+0.02%) from 90.806%
when pulling daf3930 on del_sum_waveform_top_array
into c618480 on master.

@dachengx dachengx marked this pull request as ready for review August 27, 2024 06:10
@dachengx dachengx requested a review from WenzDaniel August 27, 2024 06:11
@WenzDaniel
Copy link
Collaborator

Sorry, but I need a bit more context why are we removing this? Do we not want to store the top waveform any longer?

@WenzDaniel
Copy link
Collaborator

Ah I saw you linked a PR.

@dachengx
Copy link
Collaborator Author

Sorry, but I need a bit more context why are we removing this? Do we not want to store the top waveform any longer?

Whether store it or not depends on whether there is a field "data_top" of peaklets.

@dachengx dachengx merged commit cb34a66 into master Aug 27, 2024
8 checks passed
@dachengx dachengx deleted the del_sum_waveform_top_array branch August 27, 2024 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WenzDaniel WenzDaniel WenzDaniel approved these changes

@yuema137 yuema137 Awaiting requested review from yuema137

@MerzJohannes MerzJohannes Awaiting requested review from MerzJohannes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Option redundant
3 participants
@dachengx @coveralls @WenzDaniel