Skip to content

Commit

Permalink
BUG: Fix bug with report replacement (#11318)
Browse files Browse the repository at this point in the history
  • Loading branch information
larsoner committed Nov 11, 2022
1 parent 1c1cc3f commit 4f3ad29
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 18 deletions.
1 change: 1 addition & 0 deletions doc/changes/latest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Bugs
- Fix bug where EEGLAB channel positions were read as meters, while they are commonly in millimeters, leading to head outlies of the size of one channel when plotting topomaps. Now ``montage_units`` argument has been added to :func:`~mne.io.read_raw_eeglab` and :func:`~mne.read_epochs_eeglab` to control in what units EEGLAB channel positions are read. The default is millimeters, ``'mm'`` (:gh:`11283` by `Mikołaj Magnuski`_)
- Fix bug where computing PSD with welch's method with more jobs than channels would fail (:gh:`11298` by `Mathieu Scheltienne`_)
- Fix channel selection edge-cases in `~mne.preprocessing.ICA.find_bads_muscle` (:gh:`11300` by `Mathieu Scheltienne`_)
- Fix bug with :class:`mne.Report` with ``replace=True`` where the wrong content was replaced (:gh:`11318` by `Eric Larson`_)
- Multitaper spectral estimation now uses periodic (rather than symmetric) taper windows. This also necessitated changing the default ``max_iter`` of our cross-spectral density functions from 150 to 250. (:gh:`11293` by `Daniel McCloy`_)

API changes
Expand Down
13 changes: 5 additions & 8 deletions mne/report/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ def _get_state_params():
'baseline', 'cov_fname', 'include', '_content', 'image_format',
'info_fname', '_dom_id', 'raw_psd', 'projs',
'subjects_dir', 'subject', 'title', 'data_path', 'lang',
'fname'
'fname',
)

def _get_dom_id(self, increment=True):
Expand Down Expand Up @@ -1810,14 +1810,11 @@ def _add_or_replace(

existing_names = [element.name for element in self._content]
if name in existing_names and replace:
# Find and replace existing content, starting from the last element
for idx, content_element in enumerate(self._content[::-1]):
if content_element.name == name:
self._content[idx] = new_content
return
raise RuntimeError('This should never happen')
# Find and replace last existing element with the same name
idx = [ii for ii, element_name in enumerate(existing_names)
if element_name == name][-1]
self._content[idx] = new_content
else:
# Simply append new content (no replace)
self._content.append(new_content)

def _add_code(self, *, code, title, language, section, tags, replace):
Expand Down
25 changes: 15 additions & 10 deletions mne/report/tests/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@

def _get_example_figures():
"""Create two example figures."""
fig1 = plt.plot([1, 2], [1, 2])[0].figure
fig2 = plt.plot([3, 4], [3, 4])[0].figure
fig1 = np.zeros((2, 2, 3))
fig2 = np.ones((2, 2, 3))
return [fig1, fig2]


Expand Down Expand Up @@ -609,28 +609,33 @@ def test_remove():
assert r2.html[2] == r.html[3]


def test_add_or_replace():
@pytest.mark.parametrize('tags', (True, False)) # shouldn't matter
def test_add_or_replace(tags):
"""Test replacing existing figures in a report."""
# Note that tags don't matter, only titles do!
r = Report()
fig1, fig2 = _get_example_figures()
r.add_figure(fig=fig1, title='duplicate', tags=('foo',))
r.add_figure(fig=fig1, title='duplicate', tags=('foo',))
r.add_figure(fig=fig1, title='duplicate', tags=('bar',))
r.add_figure(fig=fig2, title='nonduplicate', tags=('foo',))
r.add_figure(fig=fig1, title='duplicate', tags=('foo',) if tags else ())
r.add_figure(fig=fig2, title='duplicate', tags=('foo',) if tags else ())
r.add_figure(fig=fig1, title='duplicate', tags=('bar',) if tags else ())
r.add_figure(fig=fig2, title='nonduplicate', tags=('foo',) if tags else ())
# By default, replace=False, so all figures should be there
assert len(r.html) == 4
assert len(r._content) == 4

old_r = copy.deepcopy(r)

# Replace last occurrence of `fig1` tagges as `foo`
r.add_figure(
fig=fig2, title='duplicate', tags=('foo',), replace=True
fig=fig2, title='duplicate', tags=('bar',) if tags else (),
replace=True,
)
assert len(r._content) == len(r.html) == 4
assert r.html[1] != old_r.html[1] # This figure should have changed
# This figure should have changed
assert r.html[2] != old_r.html[2]
# All other figures should be the same
assert r.html[0] == old_r.html[0]
assert r.html[2] == old_r.html[2]
assert r.html[1] == old_r.html[1]
assert r.html[3] == old_r.html[3]


Expand Down

0 comments on commit 4f3ad29

Please sign in to comment.