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

[MRG] Adding eye tracking to ch map #1344

Merged
merged 2 commits into from
Nov 28, 2024
Merged

Conversation

christian-oreilly
Copy link
Contributor

@christian-oreilly christian-oreilly commented Nov 27, 2024

PR Description

Very small PR just adding eye-tracking types to the channel mapping. Without this, MNE Raw objects with eye-tracking data cannot be exported to BIDS. Exporting with this change seems to fix the bug. We also had to make sure there were not NaNs in the raw objects using

def fillna(raw, fill_val=0):
    return mne.io.RawArray(np.nan_to_num(raw.get_data(), nan=fill_val), raw.info)

but I did not include this in the PR as it seems like a larger issue.

Related to #1342. Not sure if enough to close that issue.

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

  • All comments are resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • New contributors have been added to CITATION.cff
  • PR description includes phrase "closes <#issue-number>"

Copy link

welcome bot commented Nov 27, 2024

Hello! 👋 Thanks for opening your first pull request here!
Please read the contributor guide, and please follow the steps outlined in the "Instructions for first-time contributors" section therein. ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@christian-oreilly
Copy link
Contributor Author

christian-oreilly commented Nov 27, 2024

Let me know if the change in this PR is something you want to merge. If so, I'll go through the checklist to get it ready to merge.

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.42%. Comparing base (5d40f8d) to head (c8292b4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1344   +/-   ##
=======================================
  Coverage   97.42%   97.42%           
=======================================
  Files          40       40           
  Lines        8883     8883           
=======================================
  Hits         8654     8654           
  Misses        229      229           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sappelhoff
Copy link
Member

Thanks for the contribution. I think we can merge this as a first step towards a more thorough eye tracking support. Please add a changelog entry and yourself to CITATION.cff as described here: https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors

@christian-oreilly christian-oreilly changed the title Adding eye tracking to ch map [MRG] Adding eye tracking to ch map Nov 27, 2024
@christian-oreilly
Copy link
Contributor Author

@sappelhoff I think the contrib info is OK now. You should be good to merge whenever you want.

@sappelhoff sappelhoff merged commit a65941f into mne-tools:main Nov 28, 2024
24 checks passed
Copy link

welcome bot commented Nov 28, 2024

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

@sappelhoff
Copy link
Member

Thanks @christian-oreilly

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.

2 participants