no 'ch_names' in annotations -> Error

[eioe]

I'm using pybv (v0.7.4) via MNE (v1.1.1) to concatenate and rewrite raw BV files.
It raises a Key error: ch_names and stops. It seems the culprit is here in _export.py.
I checked my raw file in MNE, and its annotations do indeed not contain an entry for ch_names. Did not dig deeper into the MNE code to find out what ch_names mean in the context of annotations (not directly obvious to me).
Is that an issue with my files or could _export.py check for the existence of the key first and skip it if not present (dunno if this will break things downstream)?

[sappelhoff]

Thanks for the report, this is indeed possible if your annotations were somehow created before the ch_names key was added to Annotations ... those objects of course don't have such a key. The fix you suggest sounds good, do you want to prepare a pull request to fix it @eioe?

[eioe]

Sure, I'll try to get to it today or tomorrow.

[sappelhoff]

great, please make sure to read: https://github.com/bids-standard/pybv/blob/main/.github/CONTRIBUTING.md

especially the part about pre-commit hooks. :-)

[eioe]

thanks for the hint. Do I have to do something specific (ie., going beyond what is explicitely mentioned in CONTRIBUTING.md) or are the hooks included in the test suite?

[sappelhoff]

the hooks are included here: https://github.com/bids-standard/pybv/blob/main/.pre-commit-config.yaml

you get them automatically when cloning the repo and running pre-commit install

[eioe]

That's my first shot. Feel free to refactor/feedback.
Tests pass. However, if I run it on my data, I get another error (which I assume is independent, but I cannot make sense of it yet):

io.pv now throws this:

UnboundLocalError: local variable 'max_event_descr' referenced before assignment

[sappelhoff]

what do you mean by io.pv? Can you provide more code for context?

Also, note how in your PR I edited the "description" to closes #99 --> if you use closes X or fixes X, this will make GitHub automatically close the respective issue once the PR is merged.

[eioe]

sorry, that should have been io.PY.

Here's the traceback:

Traceback

Output exceeds the [size limit](command:workbench.action.openSettings?[). Open the full output data [in a text editor](command:workbench.action.openLargeOutput?420821b4-4b3d-4e35-b670-c47ebf7bcc93)
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
/u/fklotzsche/ptmp_link/Experiments/vrstereofem/vrstereofem_analysis/Sketchpad/test_pybv.py in <cell line: 18>()
   [18](file:///u/fklotzsche/ptmp_link/Experiments/vrstereofem/vrstereofem_analysis/Sketchpad/test_pybv.py?line=17) raw2 = mne.io.read_raw_brainvision(fname2, preload=True, 
   [19](file:///u/fklotzsche/ptmp_link/Experiments/vrstereofem/vrstereofem_analysis/Sketchpad/test_pybv.py?line=18)                                 verbose=False)
   [21](file:///u/fklotzsche/ptmp_link/Experiments/vrstereofem/vrstereofem_analysis/Sketchpad/test_pybv.py?line=20) mne.concatenate_raws([raw1, raw2])
---> [22](file:///u/fklotzsche/ptmp_link/Experiments/vrstereofem/vrstereofem_analysis/Sketchpad/test_pybv.py?line=21) mne.export.export_raw(Path(eeg_data, 'test.vhdr'), raw1)

File <decorator-gen-578>:12, in export_raw(fname, raw, fmt, physical_range, add_ch_type, overwrite, verbose)

File ~/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_export.py:63, in export_raw(fname, raw, fmt, physical_range, add_ch_type, overwrite, verbose)
   [61](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_export.py?line=60) elif fmt == 'brainvision':
   [62](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_export.py?line=61)     from ._brainvision import _export_raw
---> [63](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_export.py?line=62)     _export_raw(fname, raw, overwrite)

File ~/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_brainvision.py:19, in _export_raw(fname, raw, overwrite)
   [17](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_brainvision.py?line=16) if ext != ".vhdr":
   [18](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_brainvision.py?line=17)     fname = fname.replace(ext, ".vhdr")
---> [19](file:///u/fklotzsche/conda-envs/vr2fem/lib/python3.10/site-packages/mne/export/_brainvision.py?line=18) _export_mne_raw(raw=raw, fname=fname, overwrite=overwrite)

File /raven/u/fklotzsche/Software/pybv/pybv/_export.py:90, in _export_mne_raw(raw, fname, events, overwrite)
   [87](file:///raven/u/fklotzsche/Software/pybv/pybv/_export.py?line=86) ref_ch_names = None
   [89](file:///raven/u/fklotzsche/Software/pybv/pybv/_export.py?line=88) # write to BrainVision
---> [90](file:///raven/u/fklotzsche/Software/pybv/pybv/_export.py?line=89) write_brainvision(
   [91](file:///raven/u/fklotzsche/Software/pybv/pybv/_export.py?line=90)     data=data,
...
--> [509](file:///raven/u/fklotzsche/Software/pybv/pybv/io.py?line=508) twidth = max(3, int(np.ceil(np.log10(max_event_descr))))
  [510](file:///raven/u/fklotzsche/Software/pybv/pybv/io.py?line=509) tformat = event["type"][0] + "{:>" + str(twidth) + "}"
  [511](file:///raven/u/fklotzsche/Software/pybv/pybv/io.py?line=510) event["description"] = tformat.format(event["description"])

UnboundLocalError: local variable 'max_event_descr' referenced before assignment

sry, going to for lunch now. :) back in 30.

And thanks for the hint with closes. Didn't know that. 👍

[sappelhoff]

sorry, that should have been io.PY.

no worries -- but I also don't know what io.PY is supposed to mean 🙂 --> I don't really get hat your traceback wants to tell us. But let's just ignore that, with the info we can find that the issue is here:

pybv/pybv/io.py

Lines 500 to 511 in 1a29f74

	if iev == 0:
	max_event_descr = max(
	[1]
	+ [
	ev["description"]
	for ev in events_out
	if isinstance(ev["description"], int)
	]
	)
	twidth = max(3, int(np.ceil(np.log10(max_event_descr))))
	tformat = event["type"][0] + "{:>" + str(twidth) + "}"
	event["description"] = tformat.format(event["description"])

Apparently the iev == 0 if-clause never gets triggered, which leads to an undefined max_event_descr, ... which in turn would be needed a few lines below (=referenced beore assignment error).

And the real issue is here:

pybv/pybv/io.py

Line 440 in 1a29f74

for iev, event in enumerate(events_out):

and here:

pybv/pybv/io.py

Line 485 in 1a29f74

if event["type"] in ["Stimulus", "Response"]:

iev gets counted up from zero ... but the logic assumes/requires that the first event is "response" or "stimulus" for the code to work.

I think the fix would be to pull this whole chunk:

pybv/pybv/io.py

Lines 498 to 508 in 1a29f74

	# NOTE: We format 1 -> "S 1", 10 -> "S 10", 100 -> "S100", etc.,
	# https://github.com/bids-standard/pybv/issues/24#issuecomment-512746677
	if iev == 0:
	max_event_descr = max(
	[1]
	+ [
	ev["description"]
	for ev in events_out
	if isinstance(ev["description"], int)
	]
	)

... out of the loop and place it already above here:

pybv/pybv/io.py

Line 440 in 1a29f74

for iev, event in enumerate(events_out):

(with proper logic so that only stimulus and response events are considered)

TLDR; this is a separate bug that we need to fix. thanks for the report! I'll take care of this one ... your #100 can be merged regardless of this, once you have updated the changelog and added yourself to the authors file.

[eioe]

👍 great.
I was wondering how that if clause never gets triggered. Makes sense now. Thanks for the explanation.

btw, with io.py I was referring to the file where the error occurs (according to the traceback). 🙂

[sappelhoff]

ahhh, thanks 🤦‍♂️ makes sense!

[sappelhoff]

@eioe once you finish the todos in #100 I can merge both PRs and make a patch release, so that your pipeline hopefully works.

[eioe]

thx, yes, I'm on it. 1 sec

[eioe]

@sappelhoff I think, I'm finally done. Thx for the quick support.

[sappelhoff]

thanks @eioe! -- could you checkout the main branch and pull the upstream changes and then see if your other problem is also resolved?

[eioe]

Yep, looks good. Also no other problems now.
Thanks again for helping out so fast! 🙏