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] FIX: Support commas in BrainVision channel names, event types, and event descriptions #8492
Conversation
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.
Should we fix it?
Yes I think fixing the channel names and the event type and descriptions would be good
491c283
to
0a47b37
Compare
Would this PR be eligible for a backport? It may come in handy for the pybv package. |
Sure but we just cut 0.21.1 yesterday so it might be a few weeks or so until we do a 0.21.2 |
indeed, on Windows, the temporary file-replacement for testing is not working as expected: The windows tests only fail with:
|
@cbrnr could you please have a look at this one and merge if you are happy? Then I can switch on the corresponding test on pybv with |
I'll merge once CIs come back green. |
Not sure what's up with the failing test. It stopped after |
I don't know either. |
I'll re-run and see if this magically resolves the issue (it often does). |
Now Travis fails at the same test ( |
It's related to #8494 I'm working on a fix. |
Co-authored-by: Clemens Brunner <clemens.brunner@gmail.com>
@cbrnr I rebased on Guillaume's fix and CIs pass now :-) |
Thanks @sappelhoff! |
This came up in bids-standard/pybv#53
In the BrainVision format channel names, event types, and event descriptions that contain a comma
,
are encoded such that the comma is written as\1
.I add a test here to show that MNE-Python does currently not adequately replace the
\1
characters to,
during reading.This is potentially broken for event type and event description as well, but I haven't looked into that yet.
Should we fix it?