Skip to content

Conversation

@mmagnuski
Copy link
Member

@mmagnuski mmagnuski commented Jul 5, 2021

Fixes #9526 and #9511. I'll add the example posted by @agramfort as a test later.

@rob-luke
Copy link
Member

rob-luke commented Jul 5, 2021

Could this possibly fix/close #8291 too?

@mmagnuski
Copy link
Member Author

Unfortunatelly #8291 is much more complex, this PR is a simple bug-fix. For #8291 we would first need some logic for detecting sensor groups.

@rob-luke
Copy link
Member

rob-luke commented Jul 6, 2021

Thanks for the clarification @mmagnuski, I was just hoping for a magic fix 😄

@agramfort
Copy link
Member

@mmagnuski can you just a what's new entry? thx for the quick reaction to fix this one !

@mmagnuski
Copy link
Member Author

No problem! what's new entry added, waiting for the CIs.

@mmagnuski
Copy link
Member Author

One CI task (Azure Main Ultraslow) timed out, rest is green.

evoked.set_montage(ten_five)

evoked.plot_topomap(sphere=0.05, res=8)
plt.close('all')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI this is no longer necessary as we have a magic autouse mpl fixture that calls plt.close

mne-python/mne/conftest.py

Lines 131 to 137 in 2e200aa

@pytest.fixture(autouse=True)
def close_all():
"""Close all matplotlib plots, regardless of test status."""
# This adds < 1 µS in local testing, and we have ~2500 tests, so ~2 ms max
import matplotlib.pyplot as plt
yield
plt.close('all')

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, good, I just followed what other tests there used (and actually thought about a decorator like that), I can change this in the next PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am slowly eliminating the ones at the end of functions as I see them. (The ones in the middle of tests are still useful to prevent too many MPL windows from popping up, eating memory, and possibly emitting warnings about 20+ windows.)

@larsoner larsoner merged commit 3c60678 into mne-tools:main Jul 6, 2021
@larsoner
Copy link
Member

larsoner commented Jul 6, 2021

Thanks @mmagnuski !

@mmagnuski
Copy link
Member Author

Thanks @larsoner @agramfort! 🚀

larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 6, 2021
* upstream/main:
  MAINT: Avoid VTK 9.0.2 [skip actions] [skip circle] (mne-tools#9530)
  MRG: Test get_montage on fNIRS data (mne-tools#9524)
  FIX: fix topo plot when large channel distance gives rise to invalid values for arcsin (mne-tools#9528)
  use Polynomial.fit for poly fitting (mne-tools#9514)
larsoner added a commit to larsoner/mne-python that referenced this pull request Jul 6, 2021
* upstream/main:
  MRG, ENH: Abstracted volume registration again (mne-tools#9521)
  MAINT: Avoid VTK 9.0.2 [skip actions] [skip circle] (mne-tools#9530)
  MRG: Test get_montage on fNIRS data (mne-tools#9524)
  FIX: fix topo plot when large channel distance gives rise to invalid values for arcsin (mne-tools#9528)
  use Polynomial.fit for poly fitting (mne-tools#9514)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash topomap with only 4 channels from 10-5 system

4 participants