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

multiple bugs in pupil and gaze preprocessing #678

Closed
SerenXia opened this issue Apr 3, 2024 · 1 comment · Fixed by #705 · May be fixed by #721
Closed

multiple bugs in pupil and gaze preprocessing #678

SerenXia opened this issue Apr 3, 2024 · 1 comment · Fixed by #705 · May be fixed by #721

Comments

@SerenXia
Copy link
Contributor

SerenXia commented Apr 3, 2024

Summary

  • pspm_pupil_correct_eyelink issue:
  1. in the help text of function "pspm_pupil_correct_eyelink", it says "options.channel" should be either "numeric" or "string", but the function actually cannot take numeric input as set in the function "pspm_options" (only *Char).
  2. would like to have an option to process both eyes (currently users have to loop for both eyes if they still want to combine pupil data later).
  • pspm_load_data issue:
  1. this function cannot load preprocessed or combined pupil channels properly, because we have taken out all "_pp" channels (in the preprocessing functions, but not in the function "pspm_init"), but in the within-function function "get_chans_to_load_for_pupil", it is still parsing "_pp" as preprocessed channels or combined channels.
  2. additionally, when loading pupil_l or pupil_r, the default setting is to take unprocessed channels (line 340), which conflicts with the within-function function "get_chans_to_load_for_pupil" help text (points 3 and 4). And for users, this default setting is not changeable. However, as we have taken out all "_pp" in channel types, this conflict did not influence the result, but it is very confusing.
  • pspm_find_valid_fixations issue:
  1. unmatched helptext for "options.eyes" compared to the actual valid values used in the function (i.e., "combined" vs. "all"); to be noted, in the practical analysis, setting option.eyes to be "combined" should be interpreted differently as "all".
    if "combined": I want to run the function for the combined eye data (and so combined gaze x/y channels are required; another option would be to make it similar as in "pspm_pupil_correct_eyelink", we can say that this is not possible for combined pupil data).
    if "all": I want to run the function for all eye channels.
    For the current codes, setting options.eyes to be "combined" works as if options.eyes = "all".

  2. And once more, find_valid_fixation is the only function (as far as I am aware) that takes the first channel if multiple channels with the same channel type exist, while the other functions always take the last channel.

  • pspm_gaze_pp issue:
  1. input argument unclear to users, especially "options.channel". Instead of taking "gaze_x_l/r" and "gaze_y_l/r", taking automatically both x and y coordinates for the selected eye(s) makes more sense.

Technical Info

  • PsPM version: 6.1.2
  • MATLAB version: R2022b
@dominikbach
Copy link
Contributor

All of these with one exception are fixed in the current develop branch. Feature request 2 for pspm_pupil_correct_eyelink will be implemented in a future PR.

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 a pull request may close this issue.

2 participants