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.

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

WIP: Calibration Interrupt #359

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
26 changes: 26 additions & 0 deletions expyfun/_experiment_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def __init__(self, exp_name, audio_controller=None, response_device=None,
# placeholder for extra actions to do on flip-and-play
self._on_every_flip = []
self._on_next_flip = []
self._on_every_wait = []
self._on_trial_ok = []
# placeholder for extra actions to run on close
self._extra_cleanup_fun = [] # be aware of order when adding to this
Expand Down Expand Up @@ -680,6 +681,31 @@ def call_on_every_flip(self, function):
else:
self._on_every_flip = []

def call_on_every_wait(self, function):
"""Add a function to be executed on every wait.

Parameters
----------
function : function | None
The function to call. If ``None``, all the "on every flip"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this say all the "on every wait"?

functions will be cleared.

See Also
--------
ExperimentController.call_on_next_flip

Notes
-----
See `flip_and_play` for order of operations. Can be called multiple
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look correct / relevant

times to add multiple functions to the queue. If the function must be
called with arguments, use `functools.partial` before passing to
`call_on_every_flip`.
"""
if function is not None:
self._on_every_wait.append(function)
else:
self._on_every_wait = []

def _convert_units(self, verts, fro, to):
"""Convert between different screen units"""
check_units(to)
Expand Down
45 changes: 41 additions & 4 deletions expyfun/_eyelink_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,17 @@ class EyelinkController(object):
Sample rate to use. Must be one of [250, 500, 1000, 2000].
verbose : bool, str, int, or None
If not None, override default verbose level (see expyfun.verbose).
calbration_keys : list
Keys that will trigger recalibration when check_recalibration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you expand this docstring line a bit? The reference to check_calibration needs context (i.e., it's not a parameter of this constructor, so explain what it is / where it lives)... is it referring to the new check_recalibrate method? (if so, the method name is wrong here)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, you say it should be list but you initiate it with a tuple. Will it work with any iterable? if so, say iterable instead of list


Notes
-----
The data will be saved to the ExperimentController ``output_dir``.
If this was `None`, data will be saved to the current working dir.
"""
@verbose_dec
def __init__(self, ec, link='default', fs=1000, verbose=None):
def __init__(self, ec, link='default', fs=1000, verbose=None,
calibration_key=['c']):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use mutable default arguments. Tuple is immutable so ('c',) is better here

if link == 'default':
link = get_config('EXPYFUN_EYELINK', None)
if link is not None and pylink is None:
Expand Down Expand Up @@ -189,6 +192,7 @@ def __init__(self, ec, link='default', fs=1000, verbose=None):
self._current_open_file = None
logger.debug('EyeLink: Setup complete')
self._ec.flush()
self.calibration_key = calibration_key

def _setup(self, fs=1000):
"""Start up Eyelink
Expand Down Expand Up @@ -381,6 +385,36 @@ def calibrate(self, beep=False, prompt=True):
self._start_recording()
return fname

def check_recalibrate(self, keys=None, prompt=True):
"""Compare key buffer to recalibration keys and calibrate if matched.

This function always uses the keyboard, so is part of abstraction.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand ths note.


Parameters
----------
keys : list or string
keys to check if prompt recalibration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keys to check against the (set of) key(s) that trigger calibration.

prompt : bool
Whether to show the calibration prompt or not
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether to show the calibration prompt screen before starting the calibration procedure

"""
calibrate = False
if keys is None:
check = self.calibration_key
keys = self._ec._response_handler._retrieve_keyboard_events(check)
else:
if isinstance(keys, string_types):
keys = [keys]
if isinstance(keys, list):
keys = [k for k in keys if k in self.calibration_key]
else:
raise TypeError('Calibration checking requires a string or '
' list of strings, not a {}.'
''.format(type(keys)))
if len(keys):
self.calibrate(prompt=prompt)
calibrate = True
return calibrate

def _stamp_trial_id(self, ids):
"""Send trial id message

Expand Down Expand Up @@ -505,7 +539,7 @@ def wait_for_fix(self, fix_pos, fix_time=0., tol=100., max_wait=np.inf,
"""
# initialize eye position to be outside of target
fix_success = False

calibrate = False
# sample eye position for el.fix_hold seconds
time_in = time.time()
time_out = time_in + max_wait
Expand All @@ -514,7 +548,7 @@ def wait_for_fix(self, fix_pos, fix_time=0., tol=100., max_wait=np.inf,
raise ValueError('fix_pos must be a 2-element array-like vector')
fix_pos = self._ec._convert_units(fix_pos[:, np.newaxis], units, 'pix')
fix_pos = fix_pos[:, 0]
while (time.time() < time_out and not
while (time.time() < time_out and not calibrate and not
(fix_success and time.time() - time_in >= fix_time)):
# sample eye position
eye_pos = self.get_eye_position() # in pixels
Expand All @@ -525,7 +559,10 @@ def wait_for_fix(self, fix_pos, fix_time=0., tol=100., max_wait=np.inf,
time_in = time.time()
self._ec._response_handler.check_force_quit()
self._ec.wait_secs(check_interval)

calibrate = self.check_recalibrate()
# rerun wait_for_fix if recalibrated
if calibrate:
fix_success = False
return fix_success

def maintain_fix(self, fix_pos, check_duration, tol=100., period=.250,
Expand Down
2 changes: 2 additions & 0 deletions expyfun/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,8 @@ def wait_secs(secs, ec=None):
win.dispatch_events()
if ec is not None:
ec.check_force_quit()
for function in ec._on_every_wait:
function()


def running_rms(signal, win_length):
Expand Down
6 changes: 5 additions & 1 deletion expyfun/tests/test_eyelink_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
import warnings

from expyfun import EyelinkController, ExperimentController
from expyfun._utils import _TempDir, _hide_window, requires_opengl21
from expyfun._utils import (_TempDir, _hide_window, requires_opengl21,
fake_button_press)

warnings.simplefilter('always')

Expand Down Expand Up @@ -46,6 +47,9 @@ def test_eyelink_methods():
# run much of the calibration code, but don't *actually* do it
el._fake_calibration = True
el.calibrate(beep=False, prompt=False)
fake_button_press(ec, 'c')
el.check_recalibrate(prompt=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you assert that this is True ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test gets hung up on the prompt screen. Could put in a fake_button_press if you want to test the prompt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. It currently gets stuck, or will get stuck with my suggested addition of an assert in this line preceding the el...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read too quickly, you are correct

el.check_recalibrate('k', prompt=False)
el._fake_calibration = False
# missing el_id
assert_raises(KeyError, ec.identify_trial, ec_id='foo', ttl_id=[0])
Expand Down