Skip to content

Conversation

@alexk101
Copy link
Contributor

@alexk101 alexk101 commented Feb 7, 2024

Reformats channel and detector lookup from array based to dictionary based. Removes incorrect assertions that every detector and source must have data associated with every registered optode position.

Reference issue

Example: Fixes #12429.

What does this implement/fix?

Reformats channel and detector lookup from array based to dictionary based. Removes incorrect assertions that every detector and source must have data associated with every registered optode position.

@alexk101 alexk101 requested a review from rob-luke as a code owner February 7, 2024 23:29
@welcome
Copy link

welcome bot commented Feb 7, 2024

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

@larsoner
Copy link
Member

larsoner commented Feb 8, 2024

@alexk101 can you add a doc/changes/devel/12430.bugfix.rst with a short sentence describing the fix, and add your name there with the :newcontrib: directive and URL to doc/changes/names.inc?

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexk101 at some point if there is any way you could get a tiny (1s?) recording that shows this problem and add it to mne-testing-data, then we could add a proper regression test that would fail on current main and pass on this PR. But since our current tests pass, I assume you've checked it on your files, and I'm not sure how easy it'll be to get such a file in the testing data, I think we're safe enough to merge here (and should add the additional test in a follow-up PR if we can!).

@larsoner larsoner enabled auto-merge (squash) February 14, 2024 14:56
@larsoner larsoner merged commit 8e55fef into mne-tools:main Feb 14, 2024
@welcome
Copy link

welcome bot commented Feb 14, 2024

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

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.

Snirf Fill Parsing Incorrect Assertions

2 participants