Fix #9383, removing delta in set_annotations with corresponding tests. #10351
Conversation
mne/tests/test_annotations.py
Outdated
| n_channels = 1 | ||
| sampling_freq = 100 | ||
| length_seconds = 20 | ||
|
|
||
| info = mne.create_info(n_channels, sfreq=sampling_freq, | ||
| ch_types=['eeg']) | ||
| data = np.random.randn(n_channels, sampling_freq * length_seconds) | ||
| raw = mne.io.RawArray(data, info) |
There was a problem hiding this comment.
@jevenzh you could use a pytest fixture to avoid recomputing such a raw toy data for each test. Tell us if you need help
There was a problem hiding this comment.
Oops, I indeed violated the DRY principle, thanks for the hint! I will learn about the pytest fixture and apply it here.
| @@ -687,17 +687,15 @@ def set_annotations(self, annotations, emit_warning=True, | |||
| ' which case the annotation onsets would be' | |||
| ' taken in reference to the first sample of' | |||
| ' the raw object.') | |||
|
|
|||
| delta = 1. / self.info['sfreq'] | |||
There was a problem hiding this comment.
According to the CI test results, it seems that removing this delta will affect the annotation filtering behavior. For example:
data = np.ones((1, 1000))
info = create_info(1, 1000., 'eeg')
raw = RawArray(data, info)
this will result in data duration of 0.999 instead of 1 second.
There was a problem hiding this comment.
So it seems like maybe we do need to keep delta in there for deciding which annotation durations to crop, but adjust which annotations get removed based on onset ...? Like in the case of 1 sec of data at 1000 Hz, if an annotation has onset=0 and duration = 1 it shouldn't be cropped, but one with an onset >= 1 should be removed because it actually starts (rather than ends) at the next time point?
|
@jevenzh do you think we should continue here? One way forward would be to keep the new tests, but revert the change about |
Thanks for contributing a pull request! Please make sure you have read the
contribution guidelines
before submitting.
Please be aware that we are a loose team of volunteers so patience is
necessary. Assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
Again, thanks for contributing!
Reference issue
Example: Fixes #1234.
What does this implement/fix?
Explain your changes.
Additional information
Any additional information you think is important.