Resolves #865

[k1o0]

I've refactored the function and added the following test:

exptQC = [
    {'frameQC_names': np.array(['ok', 'PMT off', 'galvos fault', 'high signal'], dtype=object),
     'frameQC_frames': np.array([0, 0, 0, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 4])},
    {'frameQC_names': np.array(['ok', 'PMT off', 'foo', 'galvos fault', np.array([])], dtype=object),
     'frameQC_frames': np.array([0, 0, 1, 1, 2, 2, 2, 2, 3, 4])},
    {'frameQC_names': 'ok',  # check with single str instead of array
     'frameQC_frames': np.array([0, 0])}
]

This will produce a new enumeration that looks like this:

   qc_values     qc_labels
0          0            ok
1          1       PMT off
2          2  galvos fault
3          3   high signal
4          4           foo
5          5       unknown

So the behaviour is this:

• ok is always 0, the other values are purely determined by the order of frameQC names first encountered;
• if values correspond to different names across bouts this will not result in an error
• all values that don't correspond to a label will be assigned the 'unknown' label
• an empty str or empty array will be assigned the same 'unknown' label instead of raising
• if names is simply an 'ok' str (instead of an array) this is handled as usual

[samupicard]

the intended behavior of _consolidate_exptQC looks good to me! Thanks for adding a test as well. Just have a separate comment about the bad_frames

[samupicard]

the frames that should be excluded from suite2p cell detection are not necessarily all frame_qc != 0. E.g. 'PMT off' or 'galvos fault' will mean those frames need to be excluded, but e.g. 'adjust spout' will not. However, this doesn't need to be figured out here - the acquirer will have written a separate badframes.mat vector specifying which frames suite2p should be excluding from analysis. So probably best to not try to impute the bad_frames variable here at all, and instead load it from disk?

[samupicard]

if loaded from disk, this badframes array will also need to be consolidated (the indexing of the badframes.mat vectors is relative to each bout)

[k1o0]

I can confirm that the bad_frames var returned by this method is not at all used. The badframes.mat files are loaded afterward, and there is an assertion that all badframes indices are a subset of the non-zero frameQC indices:

ibllib/ibllib/pipes/mesoscope_tasks.py

Lines 773 to 787 in c4418cb

	# If applicable, save as bad_frames.npy in first raw_imaging_folder for suite2p
	# badframes.mat contains QC values that do affect ROI detection (e.g. no PMT, lens artefacts)
	badframes = np.array([], dtype='i8')
	total_frames = 0
	# Ensure all indices are relative to total cumulative frames
	for m, collection in zip(all_meta, raw_image_collections):
	badframes_path = self.session_path.joinpath(collection, 'badframes.mat')
	if badframes_path.exists():
	raw_mat = loadmat(badframes_path, squeeze_me=True, simplify_cells=True)['badframes']
	badframes = np.r_[badframes, raw_mat + total_frames]
	total_frames += m['nFrames']
	if len(badframes) > 0 and use_badframes is True:
	# The badframes array should always be a subset of the frameQC array
	assert np.max(badframes) < frameQC.size and np.all(frameQC[badframes])
	np.save(raw_image_collections[0].joinpath('bad_frames.npy'), badframes)