Session module #127
Session module #127
Conversation
…ying inquiry evidence from session helper to session data structures.
…ts for session helper
There was a problem hiding this comment.
Still testing locally, but this is enough to get you rolling! Great refactoring effort here!
We should add session to the glossary in this PR - this is the latest from my last PR: https://github.com/CAMBI-tech/BciPy/blob/bids-trigger-fix/README.md#glossary
The glossary terms themselves are negotiable (ex. mode vs paradigm) but we should document them and make tickets to update as needed!
bcipy/tasks/session_data.py
Outdated
| from collections import Counter | ||
|
|
||
|
|
||
| class StimSequence: |
There was a problem hiding this comment.
We can call this an inquiry I think?
bcipy/tasks/session_data.py
Outdated
|
|
||
| def __init__(self, | ||
| stimuli: List[str], | ||
| eeg_len: int, |
There was a problem hiding this comment.
We could make this a little more generic. data_len.
There was a problem hiding this comment.
I was looking through the usage and what this is referring to is the number of channels in the EEG data. This is going to be the same value for all Inquiries, so I'm not sure it makes sense to store this here. If anything it should be a Session variable, but I'm wondering if it even pertains to sessions at all. It seems like the concern of session-related data should be around what was presented and how decisions were made. Maybe this parameter should be removed. Thoughts?
There was a problem hiding this comment.
I agree, let's remove it. I think it was used for debugging at one point.
bcipy/tasks/session_data.py
Outdated
| def __init__(self, | ||
| stimuli: List[str], | ||
| eeg_len: int, | ||
| timing_sti: List[float], |
There was a problem hiding this comment.
timing may be sufficient as a name
bcipy/tasks/session_data.py
Outdated
| target_info: List[str], | ||
| target_letter: str, | ||
| current_text: str, | ||
| copy_phrase: str, |
There was a problem hiding this comment.
This should be optional in the case of other tasks (free_spell). Perhaps, target_text?
| self.copy_phrase = copy_phrase | ||
| self.next_display_state = next_display_state | ||
|
|
||
| # TODO: refactor for multimodal; List of Evidences? |
There was a problem hiding this comment.
I like that idea. We can make another ticket for that if you'd like
bcipy/tasks/session_data.py
Outdated
| 'lm_evidence': dict(zip(alphabet, self.lm_evidence)), | ||
| 'eeg_evidence': dict(zip(alphabet, self.eeg_evidence)), | ||
| 'likelihood': likelihood, | ||
| 'most_likely': dict(Counter(likelihood).most_common(5)) |
There was a problem hiding this comment.
implement n here but rename to most_likely or something related
bcipy/tasks/session_data.py
Outdated
| def __init__(self, | ||
| save_location: str, | ||
| session_type: str = 'Copy Phrase', | ||
| paradigm: str = 'RSVP'): |
| ], | ||
| [ | ||
| "F", | ||
| 1.984226447995752 |
There was a problem hiding this comment.
Since this is a test, we could reduce this to a few inquiries? It could reduce the difficulty in looking over this data in tests
| { | ||
| "session": "data/default/test_user_001/test_user_001_RSVP_Copy_Phrase_Wed_17_Mar_2021_17hr11min05sec_-0700", | ||
| "session_type": "Copy Phrase", | ||
| "paradigm": "RSVP", |
There was a problem hiding this comment.
Wherever we write this it should be mode
| @@ -0,0 +1,2431 @@ | |||
| { | |||
| "session": "data/default/test_user_001/test_user_001_RSVP_Copy_Phrase_Wed_17_Mar_2021_17hr11min05sec_-0700", | |||
| "session_type": "Copy Phrase", | |||
There was a problem hiding this comment.
We might rename this to task
|
I like the naming suggestions. They make the code/data a lot more readable and consistent. |
| @@ -60,7 +60,7 @@ To use all the goodies locally (including the GUI and demo scripts) | |||
| If wanting the latest version from PyPi: | |||
| 1. `pip install bcipy` | |||
|
|
|||
| Alternatley, if [Make](http://www.mingw.org/) is installed, you may run the follow command to install: | |||
| Alternately, if [Make](http://www.mingw.org/) is installed, you may run the follow command to install: | |||
There was a problem hiding this comment.
Thanks for catching the typos 🙈
Overview
Refactored session data from a dict to a class-based data structure. Refactored copy phrase task and session helpers to use the new data structures.
Ticket
https://www.pivotaltracker.com/story/show/176150973
Contributions
Test