-
Notifications
You must be signed in to change notification settings - Fork 7
User guide updates bpm panel #197
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
Conversation
…e_updates_bpm_panel
…e_updates_bpm_panel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. Little things here and there, nothing big. I think this is a good opportunity to discuss if we want to keep or remove some old / currently broken and potentially useless features.
### Averages, Removal of Turns and Splitting Files | ||
|
||
!!! warning "Broken Functionality" | ||
These features [are currently broken][issue283] and we are considering whether they are actually needed. | ||
|
||
It contains BPM names and corresponding threshold which identified a BPM as faulty. | ||
The buttons on the top left side of the pane provide some features to handle the BPM data. | ||
|
||
!!! note | ||
- ++"Create Average"++ allows loading several data files too visualize their average repesentations on the same graph, which helps detecting differences or reducing noise. | ||
- ++"Remove Turns"++ can be used to cut turns from the start or the end, to focus on a specified range of the data. | ||
- ++"Split Files"++ splits the current BPM data file into N files, where N is specified in the dialog and the resulting files will have old-turns/N turns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious to get the opinion of @emaclean. I'd rather have these disappears, as stated on the linked issue we can just do that, but another way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discuss on the linked issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but it also impacts this PR so if it's a quick discussion we might settle it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still a long way to merge into the master branch ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Adds a description of the BPM panel to the Beta-Beat-GUI section.
Requires #188 to be merged first and back into this branch (will then also reduce the number of files changed)