Add buttons
Reference issue (if any)
What does this implement/fix?
Additional information
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!
Hey @WouterKroot do you still have time to finish this? I think that #12847 is going to touch a lot of code in the eyelink sub package, so it will make your life easier to merge this first! Let me know if you need any help
Hey @scott-huberty, I'll work on it this week. Let you know if I need help, thanks!
@scott-huberty, Hey Scott, I have been trying to get the button events to parse into the
def _simulate_eye_tracking_data(in_file, out_file), in test_eyelink.py
However, I'm having trouble with the "BAD_ACQ_SKIP" label. When I try and change the timestamps of the button I get:
======================================================================================== short test summary info ======================================================================================== FAILED mne/io/eyelink/tests/test_eyelink.py::test_multi_block_misc_channels[fname0] - AssertionError: assert np.str_('button_1_release') == 'BAD_ACQ_SKIP' =================================================================================== 1 failed, 13 deselected in 0.71s =====
And changing the timestamps to be within the first block of simulated data did not work. Any suggestions on how to best parse in the button events? If it's unclear let me know.
Ah ok! I think the failing unit test is this one:
https://github.com/mne-tools/mne-python/blob/b64c8231febe93926ec798c01ff01b5cb78055c3/mne/io/eyelink/tests/test_eyelink.py#L400
In that test, I originally asserted that the last annotation in the simulated raw file should be "BAD_ACQ_SKIP". That happened to be true at the time.
However, now that you’ve added button events, those button annotations might come after "BAD_ACQ_SKIP". Could you:
- Inspect the raw object to see if the last annotation(s) are now button annotations.
- Then, assuming point 1. is true, can you confirm that "BAD_ACQ_SKIP" is still the last annotation before the button annotations.
If so, we can adjust the test (there’s no reason BAD_ACQ_SKIP must be the final annotation).
From the root mne-python repo, You can run:
pytest --pdb mne/io/eyelink/tests/test_eyelink -k test_multi_block_misc_channels
When the test fails, you’ll drop into the pdb debugger. Then for example you can do something like raw.annotations.description[-10:] to inspect the last 10 annotations of that object.
Okay makes sense, and point 1 and 2 are indeed the case. I’ll try and fix tomorrow
@scott-huberty , All the button events work and test_multi_block_misc_channels also works. Tried to rebase and clean up the mess, but im still a new to git.
Problem now is that test_href_eye_events does not work, I tried changing the EYELINK_COLS in utils, I don't understand where these columns come from really. I tried to debug and found that:
Key: fixations, df columns: 10, expected: 7
Which gives the error. But changing the col_names does not help. See error below: any ideas?
Key: fixations, df columns: 10, expected: 7 F mne/io/eyelink/tests/test_eyelink.py:527: in test_href_eye_events E ValueError: Length mismatch: Expected axis has 10 elements, new values have 7 elements
@WouterKroot would it be possible to add a short description in your initial post about what this PR implements? This would make it easier for potential reviewers (and our PR template provides these fields, so don't just ignore them).
It would also be great if you rebased so that previous unrelated changes do not clutter this PR.
Hey @WouterKroot sorry that you are having trouble with rebasing. If you want, I can meet with you 1:1 so that we can do this together. I will be at the MNE-Python Office hours this Friday at 15:00 UTC, held on MNE's discord server. If you can attend, we can break off and work through the rebase. If that doesn't work for you - feel free to send me an email and we will figure out a time that works.
MNE-Python Office hours this Friday at 15:00 UTC, held on MNE's discord server.
@scott-huberty Thanks again for reaching out. I would surely appreciate your time for a little 1:1 session. I think the rebase turned out fine and the buttons work fine in the simulation and the assertion tests. I think there are just some weird things happening in the:
def _infer_col_names_for_block(block: dict) -> tuple[dict[str, list], list]:
and some strange indexing in:
def test_href_eye_events(tmp_path):
This gives me that error that:
mne/io/eyelink/tests/test_eyelink.py:527: in test_href_eye_events E ValueError: Length mismatch: Expected axis has 10 elements, new values have 7 elements
I would really appreciate it if you would be able to do some debugging with me, unfortunately 15:00 UTC is a late for me. I would be able to do it earlier? I would also be available today, have to log off around 14:30 UTC.
I think the rebase turned out fine and the buttons work fine in the simulation and the assertion tests.
Did you already push? The "Files changed" tab at the very top still shows many unrelated changes.