pybv

pybv copied to clipboard

Fixes #103. This is a big (breaking) change, I decided to tackle this as follows:

• If the passed events is an np.ndarray, the previous conversion magic is still happening (i.e. type is set to "Stimulus" and integers are converted to "S 1" etc.).
• If the passed events is a list of dicts, markers are written as is.

This means that if someone needs to write "old-style" markers with custom types (such as "Response", "Comment", etc.), they must manually create a corresponding list of dicts.

I'm not sure if we need to change anything in MNE-Python, but I don't think so. It reads descriptions and types and combines them to "Type/Description", which I think should continue to work. The events from annotations heuristics will only work for "old-style" annotations, but that's OK I think.

Feb 19 '24 12:02 cbrnr

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (e1a6c79) 100.00% compared to head (bd0bb13) 100.00%. Report is 4 commits behind head on main.

@@            Coverage Diff            @@
##              main      #118   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          634       625    -9     
=========================================
- Hits           634       625    -9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Feb 19 '24 13:02 codecov[bot]

Thanks Clemens, I will have time to review this later this week!

Feb 19 '24 13:02 sappelhoff

Here's something to consider: pybv can already export arbitrary markers as type Comment, so maybe there is no need for such a big change? Coming from MNE, the private _export_mne_raw() function already does the job (it sets the type of all markers to Comment).

Feb 19 '24 15:02 cbrnr

Coming from MNE, the private _export_mne_raw() function already does the job (it sets the type of all markers to Comment).

I assume you are referring to this?

https://github.com/bids-standard/pybv/blob/79dceaa02f6a2da77e8d5b27979ee3f378626137/pybv/_export.py#L126-L130

Here's something to consider: pybv can already export arbitrary markers as type Comment, so maybe there is no need for such a big change?

I personally am able to write all data that I want to write. I thought your argument in #103 convincing so I am fine if you really need this change. But if the changes here are not a big advantage for you, then we should just leave it as is IMHO.

Feb 22 '24 09:02 sappelhoff