-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix SourceChGain/SourceChOffset handling in BCI2k reader #13869
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
97d6925
376a7ba
3442039
682fef1
e5a62a0
c42b5eb
682cf2b
6c05e3b
c0e72a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,10 @@ | |||||||||||
|
|
||||||||||||
| import mne | ||||||||||||
| from mne.datasets import testing | ||||||||||||
| from mne.io.bci2k.bci2k import ( | ||||||||||||
| _parse_bci2k_header, | ||||||||||||
| _parse_value_with_unit, | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| data_path = testing.data_path(download=False) | ||||||||||||
| bci2k_fname = data_path / "BCI2k" / "bci2k_test.dat" | ||||||||||||
|
|
@@ -25,3 +29,21 @@ def test_read_raw_bci2k(): | |||||||||||
| assert events.ndim == 2 | ||||||||||||
| assert events.shape[1] == 3 | ||||||||||||
| assert "RawBCI2k" in repr(raw) | ||||||||||||
|
|
||||||||||||
| info_dict = _parse_bci2k_header(bci2k_fname) | ||||||||||||
| assert info_dict["params"]["SourceChOffset"] == ["0", "0"] | ||||||||||||
| assert info_dict["params"]["SourceChGain"] == ["0.1muV", "0.1muV"] | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def test_parse_value_with_unit(): | ||||||||||||
| """Test numeric token parsing with embedded unit suffixes.""" | ||||||||||||
| volt_scale = {"v": 1.0, "mv": 1e-3, "muv": 1e-6, "uv": 1e-6, "nv": 1e-9} | ||||||||||||
| assert _parse_value_with_unit("0.1muV", unit_scale=volt_scale) == (0.1, 1e-6) | ||||||||||||
| assert _parse_value_with_unit("2mV", unit_scale=volt_scale) == (2.0, 1e-3) | ||||||||||||
| assert _parse_value_with_unit("-3.5µV", unit_scale=volt_scale) == (-3.5, 1e-6) | ||||||||||||
|
|
||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @nordme , Thanks a lot for reviewing the PR , will push the changes after expanding the test fns in some time.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a reason to change the example because the changes made here are in the internal functions. |
||||||||||||
| freq_scale = {"hz": 1.0, "khz": 1e3} | ||||||||||||
| value, scale = _parse_value_with_unit("256Hz", unit_scale=freq_scale) | ||||||||||||
| assert value * scale == 256 | ||||||||||||
| value, scale = _parse_value_with_unit("0.5kHz", unit_scale=freq_scale) | ||||||||||||
| assert value * scale == 500 | ||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could get away with something much simpler, like
r"[a-zA-Z]+$"to match the unit, then cut the string before/afterresult.start()to get the numeric and unit parts.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really Sorry for late reply !

I have made this function simpler, also I have assumed if no unit is present for SamplingRate we will take Hz and for SourceChGain we will take µV, as per the BCI2000 technical Ref which states SourceChGain is the factor to convert raw AD units into µV.