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

fix merge_asof usage in read_raw_eyelink() #12481

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

dominikwelke
Copy link
Contributor

@dominikwelke dominikwelke commented Mar 4, 2024

closes #12480

the new parameter set is also not perfect:
direction=nearest can lead to duplicating samples (as a sample on an uneven ms is equally distanced to the previous and subsequent millisecond). you can see this behavior running the simplified example code in the issue.

but i think this is better than forward/backward which does not duplicate samples, but works worse with sfreqs < 500 Hz, as it can shift samples in the wrong direction

@scott-huberty
Copy link
Contributor

Thanks again! - I haven't looked at the code yet, but if merge_asof is still giving us some trouble I'm just going to crossref to my comment in the associated issue about refactoring the code to more closely the pattern in read_raw_neuralynx:

#12480 (comment)

@larsoner larsoner merged commit ae69b03 into mne-tools:main Mar 5, 2024
30 checks passed
@larsoner
Copy link
Member

larsoner commented Mar 5, 2024

Thanks @dominikwelke

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
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.

Bug: read_raw_eyelink does not read all data
3 participants