Add buttons #13287

WouterKroot · 2025-06-16T14:52:01Z

Reference issue (if any)

What does this implement/fix?

Additional information

for more information, see https://pre-commit.ci

larsoner

Looks reasonable to me! Just a few minor comments

mne/io/eyelink/_utils.py

larsoner · 2025-06-17T14:34:53Z

mne/io/eyelink/tests/test_eyelink.py

+                    f"BUTTON\t{t}\t{btn_id}\t{state}\t100\t20\t45\t45\t127.0\t"
+                    "1497.0\t5189.0\t512.5\t.............\n"
+                )
+                button_idx += 1


Looks like reasonable modifications, but later I'd expect some assertions about the raw.annotations or something for example that the right number of button events occurred etc.

@WouterKroot do you need more guidance on this part specifically?

Yes please, not sure that I follow. What do you mean by assertions, as in tests?

So if you look later in the file you see stuff like

mne-python/mne/io/eyelink/tests/test_eyelink.py

Lines 318 to 319 in b50a29a

assert raw.annotations.description[1] == "SYNCTIME"

assert raw.annotations.description[-1] == "BAD_ACQ_SKIP"

I would expect similar assert statements to work here. So some of the annotations now should have some button description, there should be a specific number that have a button description, etc. So something like the following (untested, needs to be adapted)

button_idx = [ii for ii, desc enumerate(raw.annotations.description) if "button" in desc.lower()] assert len(button_idx) == 6 # or however many you added assert_allclose(raw.annotations.onset[button_idx[0]], 1.2356, atol=1e-5) # or whatever the onset of the first simulated button press was

make sense?

And you can add to the test and test locally with

pytest mne/io/eyelink -k multi_block_misc

to make sure it passes before pushing

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

WouterKroot · 2025-06-17T14:43:23Z

Thanks for the comments, to be fair I don't really understand the eyelink asc format. Also I am new to these tests and how to efficiently push relevant changes. Please let me know if you have more suggestions and comments!

scott-huberty

Getting there! that simulate_data function was already kind of hairy, so nice start @WouterKroot.

the test_multi_block_misc_channels test was failing on your branch, so the changes I suggested should get things passing..

But you will need find these lines:

assert raw.annotations.description[-1] == "BAD_ACQ_SKIP"
assert np.isclose(raw.annotations.onset[-1], 1.001)
assert np.isclose(raw.annotations.duration[-1], 0.1)

And change them to something like

assert raw.annotations.description[-1] == "button_1_release"
# there should be 1 BAD_ACQ_SKIP annotation between our two blocks
assert raw.annotations.description[-8] == "BAD_ACQ_SKIP"
assert np.isclose(raw.annotations.onset[-8], 1.001)
assert np.isclose(raw.annotations.duration[-8], 0.1)

This is because the last annotation in this file used to be BAD_ACQ_SKIP, but now the last 6 or so annotations are the button events you've added, so our assert statements need to reflect that.

The last sticking point for me is that with this test case, I would expect the first button related annotation tone button_1_pressed but it appears to be button_1_released. Why is that?

>>> raw.annotations.description
array(['fixation', 'SYNCTIME', 'saccade', 'fixation', 'saccade',
       'fixation', 'saccade', 'fixation', 'saccade', 'BAD_ACQ_SKIP',
       'button_1_release', 'button_1_press', 'button_1_release',
       'button_1_press', 'button_1_release', 'button_1_press',
       'button_1_release'], dtype='<U16')
```

scott-huberty · 2025-06-24T00:37:28Z

mne/io/eyelink/_utils.py

+            )
+        df_dict["buttons"] = pd.DataFrame(parsed)
+    n_button = len(df_dict.get("buttons", []))
+    logger.info(f"Found {n_button} button event{_pl(n_button)} in this file.")


I'm not sure if this _pl function was supposed to add an s character in the case of multiple button events (because _pl is not defined), but I think the logging message below is good enough?

Suggested change

logger.info(f"Found {n_button} button event{_pl(n_button)} in this file.")

logger.info(f"Found {n_button} button event(s) in this file.")

scott-huberty · 2025-06-24T01:19:12Z

mne/io/eyelink/tests/test_eyelink.py

@@ -246,6 +268,22 @@ def _simulate_eye_tracking_data(in_file, out_file):
                "...\t1497\t5189\t512.5\t.............\n"
            )

+        for timestamp in np.arange(5488500, 5488600):  # 100 samples


Suggested change

for timestamp in np.arange(5488500, 5488600): # 100 samples

# And let's add some buttons events to this block

for timestamp in np.arange(7453390, 7453490): # 100 more samples

So these new lines you appended to the end of the simulated file, actually had earlier timestamps than the previous lines, which breaks things. So I'm just making sure the timestamps are increasing.

scott-huberty · 2025-06-24T01:26:42Z

mne/io/eyelink/tests/test_eyelink.py

@@ -214,7 +227,7 @@ def _simulate_eye_tracking_data(in_file, out_file):
                if event_type.isnumeric():  # samples
                    tokens[4:4] = ["100", "20", "45", "45", "127.0"]  # vel, res, DIN
                    tokens.extend(["1497.0", "5189.0", "512.5", "............."])
-                elif event_type in ("EFIX", "ESACC"):
+                elif event_type in ("EFIX", "ESACC", "BUTTON"):


Suggested change

elif event_type in ("EFIX", "ESACC", "BUTTON"):

elif event_type in ("EFIX", "ESACC"):

scott-huberty · 2025-06-24T01:29:26Z

mne/io/eyelink/tests/test_eyelink.py

+                elif event_type == "BUTTON":
+                    # simulate a button event
+                    tokens[1] = "BUTTON"  # simulate button press
+                    tokens[2] = "1"  # simulate button 1
+                    tokens[3] = "1"  # simulate button pressed
+                    tokens[4:4] = ["100", "20", "45", "45", "127.0"]
+                    tokens.extend(["1497.0", "5189.0", "512.5", "............."])
+
+                    continue


Suggested change

elif event_type == "BUTTON":

# simulate a button event

tokens[1] = "BUTTON" # simulate button press

tokens[2] = "1" # simulate button 1

tokens[3] = "1" # simulate button pressed

tokens[4:4] = ["100", "20", "45", "45", "127.0"]

tokens.extend(["1497.0", "5189.0", "512.5", "............."])

continue

Hint: This simulated eyetracking data is built off the actual eyetracking test file ~/mne_data/MNE-testing-data/eyetrack/test_eyelink_HREF.asc, which doesn't have any button events! So the code you added there will never actually be hit : )

scott-huberty · 2025-06-24T01:32:11Z

mne/io/eyelink/tests/test_eyelink.py

+    # Define your known BUTTON events
+    button_events = [
+        (5488529, 1, 0),
+        (5488532, 1, 1),
+        (5488540, 1, 0),
+        (5488543, 1, 1),
+        (5488550, 1, 0),
+        (5488553, 1, 1),
+        (5488571, 1, 0),
+    ]


Suggested change

# Define your known BUTTON events

button_events = [

(5488529, 1, 0),

(5488532, 1, 1),

(5488540, 1, 0),

(5488543, 1, 1),

(5488550, 1, 0),

(5488553, 1, 1),

(5488571, 1, 0),

]

# Define a few BUTTON press events

button_events = [

(7453400, 1, 0),

(7453410, 1, 1),

(7453420, 1, 0),

(7453430, 1, 1),

(7453440, 1, 0),

(7453450, 1, 1),

(7453460, 1, 0),

]

I am just making sure that the timestamps here line up with the adjustments I made below.

scott-huberty · 2025-06-24T14:42:05Z

mne/io/eyelink/tests/test_eyelink.py

+            while (
+                button_idx < len(button_events)
+                and button_events[button_idx][0] == timestamp
+            ):


The while statement works, but you don't really need a loop here, you can just use a conditional statement:

Suggested change

while (

button_idx < len(button_events)

and button_events[button_idx][0] == timestamp

):

if button_idx < len(button_events) and button_events[button_idx][0] == timestamp:

Wouter Kroot and others added 12 commits May 19, 2025 12:20

Added button event parsing in _utils.py

bf5b980

Added Button events in eyelink test

1dca05d

Merge branch 'main' into add_buttons

5b9e385

[pre-commit.ci] auto fixes from pre-commit.com hooks

9ded82d

for more information, see https://pre-commit.ci

formatting

8178b0f

Formatting time sort _utils for buttons

41a12cb

[pre-commit.ci] auto fixes from pre-commit.com hooks

a6624c0

for more information, see https://pre-commit.ci

shorten some strings for tests

7671ed4

[pre-commit.ci] auto fixes from pre-commit.com hooks

f7a54d1

for more information, see https://pre-commit.ci

change button write syntax

0d5f6ce

change button write syntax

5ad0f0e

[pre-commit.ci] auto fixes from pre-commit.com hooks

4e87100

for more information, see https://pre-commit.ci

larsoner reviewed Jun 17, 2025

View reviewed changes

WouterKroot and others added 3 commits June 17, 2025 16:38

Update mne/io/eyelink/_utils.py

80e929f

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Update mne/io/eyelink/_utils.py

96043cd

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

b50a29a

for more information, see https://pre-commit.ci

scott-huberty suggested changes Jun 24, 2025

View reviewed changes

scott-huberty reviewed Jun 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add buttons #13287

Add buttons #13287

WouterKroot commented Jun 16, 2025

Uh oh!

larsoner left a comment

Uh oh!

Uh oh!

Uh oh!

larsoner Jun 17, 2025

Uh oh!

larsoner Jun 17, 2025

Uh oh!

WouterKroot Jun 17, 2025

Uh oh!

larsoner Jun 17, 2025

Uh oh!

WouterKroot commented Jun 17, 2025

Uh oh!

scott-huberty left a comment

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

scott-huberty Jun 24, 2025

Uh oh!

Uh oh!

	assert raw.annotations.description[1] == "SYNCTIME"
	assert raw.annotations.description[-1] == "BAD_ACQ_SKIP"

	logger.info(f"Found {n_button} button event{_pl(n_button)} in this file.")
	logger.info(f"Found {n_button} button event(s) in this file.")

	for timestamp in np.arange(5488500, 5488600): # 100 samples
	# And let's add some buttons events to this block
	for timestamp in np.arange(7453390, 7453490): # 100 more samples

	elif event_type in ("EFIX", "ESACC", "BUTTON"):
	elif event_type in ("EFIX", "ESACC"):

Add buttons #13287

Are you sure you want to change the base?

Add buttons #13287

Conversation

WouterKroot commented Jun 16, 2025

Reference issue (if any)

What does this implement/fix?

Additional information

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WouterKroot commented Jun 17, 2025

Uh oh!

scott-huberty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!