-
Notifications
You must be signed in to change notification settings - Fork 259
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
update brainflow to 5.0.1 #1064
Conversation
Signed-off-by: Andrey Parfenov <a1994ndrey@gmail.com>
Signed-off-by: Andrey Parfenov <a1994ndrey@gmail.com>
KNN (1, "KNN", BrainFlowClassifiers.KNN), | ||
SVM (2, "SVM", BrainFlowClassifiers.SVM), | ||
LDA (3, "LDA", BrainFlowClassifiers.LDA); | ||
REGRESSION (0, "Regression", BrainFlowClassifiers.DEFAULT_CLASSIFIER); |
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.
@Andrey1994 What happened to the other classifiers?
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.
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.
It's a little silly to have a dropdown now if there is only one option. 😕
CONCENTRATION (0, "Concentration", BrainFlowMetrics.CONCENTRATION, "Concentrating"), | ||
RELAXATION (1, "Relaxation", BrainFlowMetrics.RELAXATION, "Relaxing"); | ||
CONCENTRATION (0, "Concentration", BrainFlowMetrics.MINDFULNESS, "Concentrating"), | ||
RELAXATION (1, "Relaxation", BrainFlowMetrics.RESTFULNESS, "Relaxing"); |
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.
Maybe we should update the text displayed in this dropdown. Concentration -> Mindfulness etc.
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.
I thought about it and decided to keep it as before to dont confuse anybody, its technically the same metric as before but with different name
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.
Tested and reviewed. Maybe we should make some small changes as mentioned. Core changes for this PR are good and functional.
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.
Bumped version to 5.1.1-alpha.0 and added this to Changelog
Signed-off-by: Andrey Parfenov a1994ndrey@gmail.com