Use locking/unlocking for Info.update()

[mscheltienne]

As discussed here #9867 (comment) I added support for the lock/unlock state in Info.update().

However, this does come with a performance penalty. Consider this simplified Info:

Simplified Info

import contextlib


class Info(dict):
    
    _attributes = {
        'arg1': 'no touch.',
        'arg2': 'this one neither',
        'arg3': lambda x: x}
    
    def __init__(self, *args, **kwargs):
        self._unlocked = True
        super().__init__(*args, **kwargs)
        self._unlocked = False
    
    def __setitem__(self, key, val):
        unlocked = getattr(self, '_unlocked', True)
        if key in self._attributes:
            if isinstance(self._attributes[key], str):
                if not unlocked:
                    pass
            else:
                val = self._attributes[key](val)  # attribute checker function
        else:
            pass
        super().__setitem__(key, val)
    
    @contextlib.contextmanager
    def _unlock(self):
        state = self._unlocked if hasattr(self, '_unlocked') else False
        self._unlocked = True
        try:
            yield
        except Exception:
            raise
        finally:
            self._unlocked = state
            
    def update(self, other=None, **kwargs):
        if other is not None:
            for k, v in other.items():
                self[k] = v
        for k, v in kwargs.items():
            self[k] = v

With the default update method:

%timeit test_update()
3.23 µs ± 12.9 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

With the new update method looping on the input and using __setitem__:

%timeit test_update()
4.98 µs ± 43.7 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

Do you have any better idea?

[drammock]

the performance hit is 1.75 microseconds, I think we can live with that

[larsoner]

LGTM +1 for merge once CIs come back happy

[larsoner]

Looks like there is one problem in /home/circleci/project/tutorials/simulation/80_dics.py, feel free to fix that one. Then you don't need to push with [circle full] -- CircleCI will see that the example has changed and just run that one example (it looks at the diff and decides what to run automagically if you don't tell it to do a full build).

[mscheltienne]

@larsoner Dammit... I pushed the fix before reading the message..

[larsoner]

Just do a a git commit --allow-empty -m 'Dont run all' && git push

[larsoner]

... or even better, git commit --allow-empty -m 'Dont run all [skip actions] [skip azp]' && git push

[larsoner]

(All of our CIs auto-cancel existing runs when new commits show up)

[mscheltienne]

So I did notice that your CIs cancel runs on new commits, but I am not (yet) familiar with git in command lines.. For now, every time I tried, I messed up something 😅
I'm using GitHub Desktop which does not allow for empty commits; and on this PC I'm running Windows and both commands failed to execute.. not sure where git is installed and how to get it recognized by the terminal compared to macOS..
I'm sorry, but I am going to let you cancel the run..
BTW, what are [skip actions] and [skip azp] flags doing?

[mscheltienne]

And one more unrelated point, I noticed this:

import os.path as op
import mne
from mne.datasets import sample

data_path = sample.data_path(download=False)
raw_fname = op.join(data_path, 'MEG', 'sample', 'sample_audvis_raw.fif')

raw = mne.io.read_raw(raw_fname)
info = mne.create_info(raw.ch_names, 50, raw.get_channel_types())
info.set_montage(raw.get_montage())

Raises:

ValueError: DigMontage is only a subset of info. There are 1 channel position not present in the DigMontage. The required channels are:

['EEG 053'].

Consider using inst.set_channel_types if these are not EEG channels, or use the on_missing parameter if the channel positions are allowed to be unknown in your analyses.

Guessing this dataset is missing the DigPoint for EEG 053 (set as bad channel). Is this intended?

[larsoner]

Guessing this dataset is missing the DigPoint for EEG 053 (set as bad channel). Is this intended?

It shouldn't be. More likely it's that get_montage is only returning channel positions for the good channels, which is a bug.

BTW, what are [skip actions] and [skip azp] flags doing?

Telling GitHub actions and Azure pipelines that they don't need to run. In theory the Azure style checks should run anyway (though I think we have a bug where they don't) and the only thing your commit should have changed was the doc build. I'll run style locally to make sure, and open a PR to make it so that our style checks always run...

[larsoner]

Traceback (most recent call last):
  File "/home/circleci/project/tutorials/simulation/80_dics.py", line 249, in <module>
    csd_signal = csd_morlet(epochs['signal'], frequencies=[10])
  File "<decorator-gen-152>", line 24, in csd_morlet
  File "/home/circleci/project/mne/time_frequency/csd.py", line 935, in csd_morlet
    epochs, projs = _prepare_csd(epochs, tmin, tmax, picks, projs)
  File "/home/circleci/project/mne/time_frequency/csd.py", line 1052, in _prepare_csd
    warn('Epochs are not baseline corrected or enough highpass filtered. '
  File "/home/circleci/project/mne/utils/_logging.py", line 365, in warn
    warnings.warn_explicit(
RuntimeWarning: Epochs are not baseline corrected or enough highpass filtered. Cross-spectral density may be inaccurate.

[larsoner]

... the easiest thing might be to info = read_raw_fif(...).crop(0, 1).resample(1).info

[mscheltienne]

With 50 instead of 1 Hz, but yes.. No way to now easily set the filtering frequencies in an Info instance.

[larsoner]

Thanks @mscheltienne !