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: mne.io.read_raw_fil handling of bad channels #12597

Merged
merged 6 commits into from May 2, 2024

Conversation

georgeoneill
Copy link
Contributor

Reference issue

Fixes #12594.

What does this implement/fix?

Changes the ordering that the info struct is populated in read_raw_fil so that bad channels can be read straight from the channels.tsv file without raising an error.

@georgeoneill
Copy link
Contributor Author

I think I've attempted to be simultaneously too clever and not enough here. Pandas isn't a part of the minimal requirements so I should seek an alternative method to manipulate the channels.tsv file in testing.

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.

Looks great!

@larsoner larsoner enabled auto-merge (squash) May 2, 2024 14:14
def _set_bads_tsv(chanfile, badchan):
"""Update channels.tsv by setting target channel to bad."""
data = []
with open(chanfile) as f:
Copy link
Member

Choose a reason for hiding this comment

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

can you please explicitly pass an encoding here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did in the orignal code, but ruff seems to be auto formatting them out...?

Copy link
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

see my comment please

@georgeoneill
Copy link
Contributor Author

Now there's definitely a paper trail, I put rt/wt back into the code using the text editor on GitHub, and the CI style editor on here just took them back out. Thoughts @hoechenberger?

@hoechenberger
Copy link
Member

hoechenberger commented May 2, 2024

The formatter removes those parameters because they're the defaults

but I was talking about encoding=...

otherwise the encoding used will be platform-dependent

@hoechenberger hoechenberger enabled auto-merge (squash) May 2, 2024 15:46
@hoechenberger hoechenberger merged commit 79d54dc into mne-tools:main May 2, 2024
30 checks passed
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.

mne.io.read_raw_fil crashes if bad channels are specified prior to import
3 participants