Skip to content

Commit

Permalink
Roadmap (#3)
Browse files Browse the repository at this point in the history
* DOC: add roadmap

This discusses the current limitations and quirks

* FIX: raise notimplemented error if data are binocular

- Since we know this won't work, let's just be explicit
  • Loading branch information
scott-huberty authored Mar 28, 2024
1 parent 63f7ac3 commit bee7c16
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
3 changes: 2 additions & 1 deletion docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,5 @@ It copies much of the EDF (EyeLink Data Format) reading code.

usage
contributing
API
API
roadmap
37 changes: 37 additions & 0 deletions docs/roadmap.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
Roadmap
=======

This page describes some of the limitations of the current implementation, and
items in need of discussion.

- **Support for Binocular data**:
The current implementation does not support
reading binocular data. Adding this support will not be trivial.
From a brief look, it appears that our code assumes that each call of
``edf_get_next_data`` will contain sample data (xpos, ypos, pupil size etc.)
from a unique time point. But each call to ``edf_get_next_data`` returns data for a
single eye, so in the case of binocular data, every 2 calls will return data for the
same time point, but for different eyes (left, and right).

- **Support for all sample fields**:
The current implementation only supports
reading ``xpos``, ``ypos``, and ``pupil size`` sample fields. We should add support for
reading other sample fields, such as head position, velocity, etc. This shouldn't be
too difficult, but we will want to add a way to specify which fields to read in the
API.

- **EDF Message string representation**:
The current implementation represents the
messages as byte strings (i.e. ``b'stimulus_presentation'``). This was inherited from
the original implementation in pyeparse. I'm not sure whether this was to stay
compatible with python 2, or if memory was a concern. We could just represent them as
regular Python strings, but I will wait to see if anyone provides feedback on this
before changing it.

- **Use of Named Arrays**:
The current implementation stores data in named Numpy
arrays, so that for example you can do ``edf["discrete"]["messages"]["msg"]`` to get
an array of the message strings. This was inherited from the original pyeparse
implementation. Named arrays are nice but add some complexity (and most users
probably aren't familiar with them?). I'm not sure if we should keep them, or just
use dictionaries instead. I will wait to see if anyone provides feedback on this.
2 changes: 2 additions & 0 deletions eyelinkio/io/edf/read_edf.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,8 @@ def _handle_recording_info(edf, res):
# TODO: edfapi eye constants are 1-based, ours are 0-based. Fix this in _defines?
info["eye"] = defines.eye_constants[e.eye -1]
res["eye_idx"] = e.eye - 1 # This should be 0: left, 1: right, 2: binocular
if res["eye_idx"] == 2:
raise NotImplementedError("Reading Binocular data is not yet supported.")

# Figure out sample flags
sflags = _sample_fields_available(e.sflags)
Expand Down

0 comments on commit bee7c16

Please sign in to comment.