Allow passing list of dict as argument to events parameter (=allow writing mne.annotations to VMRK)

[sappelhoff]

closes #77

this is what's needed to make pybv a "writer" in the MNE-Python export module.

It will allow writing more detailed events to VMRK (everything 100% in line with the spec).
Specifically we will support:

• writing event types other than "Stimulus" --> for now I added "Stimulus", "Response", and "Comment"
  • "Comment" is special because it will allow any string as a "description", instead of positive ints as previously required for Stimulus
• writing events pertaining to specific channels

One caveat is, that in principle the BrainVision format does not restrict the event type to "Stimulus", "Response", "Comment. --> in fact, one could have any type, see #24 (comment)

Yet in practice, we most often see:

• Stimulus
• Response
• Comment
• New Segment
• SyncStatus

Here I implement Stimulus and Response as previously for only Stimulus --> writing integer code "event descriptions".

Comment can then be used for free form strings ... I think this suffices for now and we can see whether users will need additional stuff in the future. Let's go step by step.

• add tests
• docstr documentation
• unify API to "under the hood" always use list of dict

[codecov]

Codecov Report

Merging #86 (6f55d5f) into main (4e11a71) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main       #86    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            3         3            
  Lines          431       535   +104     
==========================================
+ Hits           431       535   +104     

Impacted Files	Coverage Δ
pybv/io.py	100.00% <100.00%> (ø)
pybv/tests/test_bv_writer.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e11a71...6f55d5f. Read the comment docs.

[sappelhoff]

in general I should probably make it so that "under the hood", everything is using the "list of dict" API 🤔 --> when array is being passed, this is quickly converted inside the func

[sappelhoff]

Hey @hoechenberger @cbrnr @adam2392 note this feature :-)

I hope this will allow us to add pybv as an "export" module to mne-python soon.

Feel free to review and let me know if you have suggestions for improvements.

Test failures are unrelated / due to some Internet issue

[cbrnr]

I didn't run and test your changes myself, but I assume that your tests cover these additions. Apart from my comments, one of the issues is the use of single or double backticks, which is currently inconsistent.

Furthermore, I was wondering if passing a pandas.DataFrame to the events parameter might be a good idea. I'd think that it would be easier to create and maintain than a list of dicts. I'm not saying that we should get rid of the list of dicts, but we could support dataframes as another option.

[hoechenberger]

Furthermore, I was wondering if passing a pandas.DataFrame to the events parameter might be a good idea. I'd think that it would be easier to create and maintain than a list of dicts. I'm not saying that we should get rid of the list of dicts, but we could support dataframes as another option.

+1 to that.

[sappelhoff]

Furthermore, I was wondering if passing a pandas.DataFrame to the events parameter might be a good idea. I'd think that it would be easier to create and maintain than a list of dicts. I'm not saying that we should get rid of the list of dicts, but we could support dataframes as another option.

Fine by me if someone wants to add this in a future PR, we'd need to then add pandas as a dependency, which is okay.

[sappelhoff]

Thanks for the review @cbrnr I addressed all your comments except the one for "single" precision, where I am unsure what to do.

[cbrnr]

Great! Forget about it, it's just that single-precision floats can represent 7 decimal places (at least), not 6.

[adam2392]

Looked good to me, but I didn't thoroughly look thru everything. Just skimmed.

[sappelhoff]

Thanks for the reviews