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: gracefully handle bad patient info in edf reader #12968

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

scott-huberty
Copy link
Contributor

@scott-huberty scott-huberty commented Nov 14, 2024

I have EEG data that I saved to EDF with MNE a long time ago (maybe 2 years ago). Reading that file back used to work, but I believe a regression/bug was introduced in #11952 . Specifically this block:

key, value = info.split("=")
if key in ["weight", "height"]:
patient[key] = float(value)
elif key in ["hand"]:
patient[key] = int(value)

This code logic assumed that patient hand info would be something like "hand=1" or "hand=2" but in my case it was "hand=None" which caused a ValueError because you cannot do int('None').

What does this implement/fix?

This PR just wraps things in a try-except block to gracefully handle weird cases like mine.

Additional information

I really don't know much about the EDF format, so cc'ing @paulroujansky as the author of #11952. LMK if you think this looks reasonable to you!.

Also - Perhaps no change log necessary?

Bug introduced in mne-tools#11952

Code assumed that hand info would be something like "hand='0'" or "hand='1'" but in my case it was "hand='None'" which caused an error because you cannot do int('None').

This just wraps things in a try-except block
@cbrnr cbrnr merged commit ede7f01 into mne-tools:main Nov 15, 2024
28 checks passed
@cbrnr
Copy link
Contributor

cbrnr commented Nov 15, 2024

Thanks @scott-huberty!

@scott-huberty scott-huberty deleted the edf_patient_value_fix branch November 15, 2024 16:18
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.

3 participants