MRG: Add convenience function to "prepare" emptyroom raw file for maxwell_filter

[hoechenberger]

Fixes #10533

This is a first quick draft, I'm looking for feedback.

cc @agramfort

[hoechenberger]

This is ready for review.

[agramfort]

Return a copy then

[hoechenberger]

Qt test failure is unrelated. This is good to merge from my end.

[hoechenberger]

Rendered API docs

https://output.circle-artifacts.com/output/job/b3b16b20-50ea-482c-b438-f3b4a9bd0548/artifacts/0/dev/generated/mne.preprocessing.maxwell_filter_prepare_emptyroom.html#mne-preprocessing-maxwell-filter-prepare-emptyroom

[hoechenberger]

I just added parameters to allow the user to decide whether or not to copy over meas_date and annotations, even though a single param (just allowing for a transfer of annots, which would implicitly alter the meas_date) might be sufficient?

I wasn't sure what to do about _cropped_samp – do we only need to adjust it if we change meas_date?

Tests are still missing.

WDYT?

[larsoner]

I wasn't sure what to do about _cropped_samp – do we only need to adjust it if we change meas_date?

Changing this changes raw.first_samp.

You need this to match 1) if you're doing movement compensation, or 2) if you're going to raw_er.set_annotations(...) afterward with annotations that match raw's timing (e.g., from annotate_movement).

If you want to raw_er.set_annotations with annotations that match raw_er's original timing, you need the meas_date and first_samp not to change.

[hoechenberger]

You need this to match 1) if you're doing movement compensation, or 2) if you're going to raw_er.set_annotations(...) afterward with annotations that match raw's timing (e.g., from annotate_movement).

If you want to raw_er.set_annotations with annotations that match raw_er's original timing, you need the meas_date and first_samp not to change.

So this is a problem with my current proposal, then?

Because we couldn't allow for keeping the ER annotations and ensuring movement compensation would work as expected at the same time? 🤔

[larsoner]

Because we couldn't allow for keeping the ER annotations and ensuring movement compensation would work as expected at the same time?

I think we can. For example if there are known really bad sections of ER data you don't want to keep them (e.g., for cov calc), using annotations='union' (new option) could make sense in this case. This is what I would use if I had such data, and I think it's a sensible default -- it creates some mismatch between which head positions (and regularization/virtual sensor processing) effectively get captured by compute_covariance down the line, but should be a less bad choice than including a segment of very bad empty room data, or than excluding usable segments of raw data. It's all about tradeoffs at this point (just like picking info['bads'] versus setting segments BAD_ using annotations!) so I think we need to give people options here.

[hoechenberger]

@larsoner Could you please take a look at my latest commit 971a519?
My idea was to allow for a transfer of annotations without forcing the measurement date to be adjusted.

Some tests are failing and I don't have the time to look into this right now, but I wanted to ask what you think about this approach in general?

[hoechenberger]

Note to self (I don't want to push changes while @larsoner is potentially working on something):

• Remove the ability to pass a list of bads. If a user wants to have a specific list of bads, they can set them before calling this function, and then pass bads='keep'. This keeps this function smaller and requires less testing

[larsoner]

Feel free to push, I don't have any in-progress changes currently

[larsoner]

@hoechenberger I simplified the logic a bit, split the test for speed purposes, and added a new param. Can you look and see what you think?

[hoechenberger]

Will do! We can talk tomorrow during the dev meeting

[larsoner]

We can talk tomorrow during the dev meeting

From my end this is feature complete and ready to go, hopefully you agree and can just merge. But yes if you think there is something wrong or more to change then we can discuss

[hoechenberger]

LGTM!

[hoechenberger]

Thanks @larsoner!