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

Read_raw_eyelink assumes STATUS data are always present #11809

Closed
scott-huberty opened this issue Jul 18, 2023 · 3 comments · Fixed by #11823
Closed

Read_raw_eyelink assumes STATUS data are always present #11809

scott-huberty opened this issue Jul 18, 2023 · 3 comments · Fixed by #11823
Labels

Comments

@scott-huberty
Copy link
Contributor

Description of the problem

Per the discussion in this thread: https://mne.discourse.group/t/problem-with-read-raw-eyelink/7243/9

read_raw_eyelink (which reads ASCII files in a tabular structure), assumes that a certain column that contains warnings (tracker droput etc) for each sample is always present. As the the researcher in the forum has shown, this is not the case for their data.

The info in this column isn't currently used by read_raw_eyelink, so we could change the reader to disregard the column entirely. Or, if we think we will enhance the reader in the future to use the information in this column, we could try to make it smart enough to emit a warning when the column isn't present.

I think I will shift my attention this week to this bug and #11758, since improving this reader and adding more tests is something that we wanted to complete this summer.

Steps to reproduce

Download the file at the link below and pass it to run `mne.io.read_raw_eyelink`

Link to data

https://drive.google.com/file/d/1QQbimGsPpc58WgJCQ91K9AyQ2oveNzpA/view?usp=sharing

Expected results

To read the file successfully

Actual results

from mne.io import read_raw_eyelink
read_raw_eyelink("./test.asc")

ValueError                                Traceback (most recent call last)
Cell In[3], line 1
----> 1 read_raw_eyelink("./test_noflags.asc")

File ~/github_repos/mne-python/mne/io/eyelink/eyelink.py:125, in read_raw_eyelink(fname, preload, verbose, create_annotations, apply_offsets, find_overlaps, overlap_threshold, gap_description)
     67 """Reader for an Eyelink .asc file.
     68 
     69 Parameters
   (...)
    121 ``'BAD_ACQ_SKIP'``.
    122 """
    123 fname = _check_fname(fname, overwrite="read", must_exist=True, name="fname")
--> 125 raw_eyelink = RawEyelink(
    126     fname,
    127     preload=preload,
    128     verbose=verbose,
    129     create_annotations=create_annotations,
    130     apply_offsets=apply_offsets,
    131     find_overlaps=find_overlaps,
    132     overlap_threshold=overlap_threshold,
    133     gap_desc=gap_description,
    134 )
    135 return raw_eyelink

File <decorator-gen-316>:12, in __init__(self, fname, preload, verbose, create_annotations, apply_offsets, find_overlaps, overlap_threshold, gap_desc)

File ~/github_repos/mne-python/mne/io/eyelink/eyelink.py:229, in RawEyelink.__init__(self, fname, preload, verbose, create_annotations, apply_offsets, find_overlaps, overlap_threshold, gap_desc)
    227 sfreq = _get_sfreq(self._event_lines["SAMPLES"][0])
    228 col_names, ch_names = self._infer_col_names()
--> 229 self._create_dataframes(
    230     col_names, sfreq, find_overlaps=find_overlaps, threshold=overlap_threshold
    231 )
    232 info = self._create_info(ch_names, sfreq)
    233 eye_ch_data = self.dataframes["samples"][ch_names]

File ~/github_repos/mne-python/mne/io/eyelink/eyelink.py:508, in RawEyelink._create_dataframes(self, col_names, sfreq, find_overlaps, threshold)
    505 first_samp = self._event_lines["START"][0][0]
    507 # dataframe for samples
--> 508 self.dataframes["samples"] = pd.DataFrame(
    509     self._sample_lines, columns=col_names["sample"]
    510 )
    511 if "HREF" in self._rec_info:
    512     pos_names = (
    513         EYELINK_COLS["pos"]["left"][:-1] + EYELINK_COLS["pos"]["right"][:-1]
    514     )

File /usr/local/Caskroom/miniconda/base/envs/mnedev/lib/python3.11/site-packages/pandas/core/frame.py:782, in DataFrame.__init__(self, data, index, columns, dtype, copy)
    780     if columns is not None:
    781         columns = ensure_index(columns)
--> 782     arrays, columns, index = nested_data_to_arrays(
    783         # error: Argument 3 to "nested_data_to_arrays" has incompatible
    784         # type "Optional[Collection[Any]]"; expected "Optional[Index]"
    785         data,
    786         columns,
    787         index,  # type: ignore[arg-type]
    788         dtype,
    789     )
    790     mgr = arrays_to_mgr(
    791         arrays,
    792         columns,
   (...)
    795         typ=manager,
    796     )
    797 else:

File /usr/local/Caskroom/miniconda/base/envs/mnedev/lib/python3.11/site-packages/pandas/core/internals/construction.py:498, in nested_data_to_arrays(data, columns, index, dtype)
    495 if is_named_tuple(data[0]) and columns is None:
    496     columns = ensure_index(data[0]._fields)
--> 498 arrays, columns = to_arrays(data, columns, dtype=dtype)
    499 columns = ensure_index(columns)
    501 if index is None:

File /usr/local/Caskroom/miniconda/base/envs/mnedev/lib/python3.11/site-packages/pandas/core/internals/construction.py:840, in to_arrays(data, columns, dtype)
    837     data = [tuple(x) for x in data]
    838     arr = _list_to_arrays(data)
--> 840 content, columns = _finalize_columns_and_data(arr, columns, dtype)
    841 return content, columns

File /usr/local/Caskroom/miniconda/base/envs/mnedev/lib/python3.11/site-packages/pandas/core/internals/construction.py:937, in _finalize_columns_and_data(content, columns, dtype)
    934     columns = _validate_or_indexify_columns(contents, columns)
    935 except AssertionError as err:
    936     # GH#26429 do not raise user-facing AssertionError
--> 937     raise ValueError(err) from err
    939 if len(contents) and contents[0].dtype == np.object_:
    940     contents = convert_object_array(contents, dtype=dtype)

ValueError: 10 columns passed, passed data had 9 columns

Additional information

See link to the discourse thread.

@larsoner
Copy link
Member

If we don't use it currently I would discard it for now with a code comment mentioning its presence but (current) lack of use by us, mentioning what you do here about how we could someday make the code smarter to handle its presence properly

@scott-huberty
Copy link
Contributor Author

I think this fix will require some refactoring (which I think we should do anyways per #11758 ). Is it okay with you if I bundle a PR for #11758 and this bug fix together?

@larsoner
Copy link
Member

Sure

@scott-huberty scott-huberty changed the title Read_raw_eyelink assumes a column Read_raw_eyelink assumes STATUS data are always present Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants