Divider in brainvision files.

[jaeilepp]

rf. mailing list.

The problem was that with long channel names the divider is single space and messes the split. It would be nice if someone with more experience with brainvision files would confirm this doesn't break anything.

[palday]

The most likely place for this to break is if the BV format (either version) allows spaces in channel names (I don't know).
@teonbrooks @jona-sassenhagen Do you know ? I can try to dig into this further when working on my own BV PR for data orientation, but I'm already behind on getting that done ....

[palday]

Instead of having separate 1.0 and 2.0 strings, it might make sense to have an iterable of supported numbers and loop over that because the initial part of the string doesn't change. This would make supporting additional version numbers in the future easier.

[jaeilepp]

It looks like the version actually has nothing to do with this, but the header field can be used to find out if the unit is in the data.

The most likely place for this to break is if the BV format (either version) allows spaces in channel names (I don't know).

That's a good point. If that's the case, it's going to need some serious heuristics for this to work.

[codecov-io]

Codecov Report

Merging #4354 into master will increase coverage by <.01%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master    #4354      +/-   ##
==========================================
+ Coverage   86.07%   86.07%   +<.01%     
==========================================
  Files         346      346              
  Lines       63059    63065       +6     
  Branches     9691     9693       +2     
==========================================
+ Hits        54275    54281       +6     
  Misses       6115     6115              
  Partials     2669     2669

[larsoner]

Instead of having separate 1.0 and 2.0 strings, it might make sense to have an iterable of supported numbers and loop over that because the initial part of the string doesn't change.

Yes I agree, @jaeilepp feel free to generalize it or we can do it once we hit a third use case.

I have no idea how the channels-with-spaces could work, hopefully they disallow it.

LGTM +1 for merge

[agramfort]

merge if it works for people

thx

[palday]

Go for merge and we can generalize the BV strings once we hit a third use case.

[larsoner]

Thanks @jaeilepp and @palday for looking