Calibration task refactor #330
Conversation
… associated timing test
There was a problem hiding this comment.
Should this be an enum?
| colors: List[str] | ||
|
|
||
| @property | ||
| def target(self) -> str: |
There was a problem hiding this comment.
Some documentation on the expected stimuli would be helpful. It seems like it would expect target fixation inquiry_stimuli?
| """Symbols used in the calibration""" | ||
| return self._symbol_set | ||
|
|
||
| def name(self) -> str: |
There was a problem hiding this comment.
Seems like we might throw a not implemented error if name wasn't defined?
bcipy/task/base_calibration.py
Outdated
| """Trigger Type. | ||
|
|
||
| This method is passed to convert_timing_triggers to properly assign TriggerTypes | ||
| to the timing of stimuli presented. |
There was a problem hiding this comment.
Would be helpful to document the assumptions here.
target prompt, fixation, stimuli (nontarget or target; dep on target
| # Allow for some additional data to be collected for later processing | ||
| self.wait() | ||
|
|
||
| def cleanup(self) -> None: |
There was a problem hiding this comment.
This could throw a not implemented error. We can add that in the future once we add cleanup to these tasks more explicitly though.
There was a problem hiding this comment.
Yeah, I think cleanup may be optional for many tasks so I prefer to leave this empty by default.
Overview
Refactored calibration tasks to pull out common workflow into a parent task. In the process, added session logging to Matrix and RSVP tasks.
Ticket
https://www.pivotaltracker.com/story/show/187637041
https://www.pivotaltracker.com/story/show/187636806
https://www.pivotaltracker.com/story/show/187636860
Contributions
Test