Skip to content

Conversation

@majiayu000
Copy link

Summary

This PR fixes #13567

Changes

  • mne/io/eyelink/_utils.py
  • mne/io/eyelink/tests/test_eyelink.py

Fix TypeError when read_raw_eyelink() processes .asc files with multiple
recording segments where a segment starts with null data. The error occurred
because _drop_status_col assumed all column values were strings, but pandas
fills missing columns with None when sample lines have varying column counts.

The fix checks for None values and looks for the first non-None value in the
column to determine if it's a status column. If all values are None, the
column is dropped.

Closes mne-tools#13567

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: majiayu000 <[email protected]>
Comment on lines 563 to 564
# Handle None values - can occur in files with multiple recording segments
# where a segment starts with null data. See gh-13567.
Copy link
Contributor

Choose a reason for hiding this comment

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

It has nothing to do with multi-block data but blocks that start with fewer columns than after

status_cols.append(col)
continue
first_val = non_null.iloc[0]
if first_val[0].isnumeric():
Copy link
Contributor

Choose a reason for hiding this comment

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

if first_val is a float it is not subscriptable and will cause error

Comment on lines +566 to +570
# Check if there's any non-None value to determine column type
non_null = samples_df[col].dropna()
if len(non_null) == 0:
# All values are None, drop this column
status_cols.append(col)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend using first_valid_index = samples_df[col].first_valid_index() as suggested in my issue comment. first_valid_index will be None when the entire column is empty. https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.first_valid_index.html. The logic here seems a bit redundant

@qian-chu
Copy link
Contributor

Hi @majiayu000, thanks for putting together the PR. However, I have some comments on the etiquette and contents of the PR.
First, the PR referred to #13567, where I created and explicitly indicated I'm happy to init a fix PR myself. While there is of course no restriction preventing others from adapting my code and contributing, a brief communication beforehand would have been appreciated.
Second, I am concerned about the technical aspects of the PR, particularly since it appears to be heavily AI-generated based on the commit description. As noted above, the implementation seems to misunderstand the actual cause of the bug, which affects my confidence in the correctness of the solution. If you wish to continue working on the PR, please review it with human eyes and test it using real Eyelink data.

- Update comment to accurately describe the issue as "blocks that start
  with fewer columns than after" instead of "multiple recording segments"
- Add isinstance check before calling isnumeric() to handle float values

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@majiayu000
Copy link
Author

Thank for the suggestion. I will close this pr :)

@majiayu000 majiayu000 closed this Dec 29, 2025
@majiayu000 majiayu000 deleted the fix-13567-readraweyelink-fails-to-find-a-1229-0110 branch December 29, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

read_raw_eyelink() fails to find and drop status column

2 participants