Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Test circle #10506

Merged
merged 3 commits into from
Apr 8, 2022
Merged

CI: Test circle #10506

merged 3 commits into from
Apr 8, 2022

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Apr 8, 2022

CicleCI is failing on main, let's see what the actual error is.

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

Failure replicated!

@GuillaumeFavelier can you replicate? I suspect this might be due to #10430

@GuillaumeFavelier
Copy link
Contributor

GuillaumeFavelier commented Apr 8, 2022

can you replicate?

Yes I can replicate on this PR. I'm on it 👍

generating gallery for auto_tutorials/inverse... [ 27%] 30_mne_dspm_loreta.py                                                                                                                
Traceback (most recent call last):
  File "/home/guillaume/source/anaconda3/envs/mne-py38/lib/python3.8/site-packages/sphinx/events.py", line 101, in emit
    results.append(listener.handler(self.app, *args))
  File "/home/guillaume/source/anaconda3/envs/mne-py38/lib/python3.8/site-packages/sphinx_gallery/gen_gallery.py", line 514, in generate_gallery_rst
    ) = generate_dir_rst(
  File "/home/guillaume/source/anaconda3/envs/mne-py38/lib/python3.8/site-packages/sphinx_gallery/gen_rst.py", line 402, in generate_dir_rst
    intro, title, cost = generate_file_rst(
  File "/home/guillaume/source/anaconda3/envs/mne-py38/lib/python3.8/site-packages/sphinx_gallery/gen_rst.py", line 1069, in generate_file_rst
    clean_modules(gallery_conf, fname, 'after')
  File "/home/guillaume/source/anaconda3/envs/mne-py38/lib/python3.8/site-packages/sphinx_gallery/scrapers.py", line 596, in clean_modules
    reset_module(gallery_conf, fname, when=when)
  File "/home/guillaume/source/mne-python/doc/conf.py", line 340, in __call__
    _assert_no_instances(Brain, when)  # calls gc.collect()
  File "/home/guillaume/source/mne-python/mne/utils/misc.py", line 378, in _assert_no_instances
    assert n == 0, f'\n{n} {cls.__name__} @ {when}:\n' + '\n'.join(ref)
AssertionError: 
1 Brain @ mne/conf.py:Resetter.__call__:after:30_mne_dspm_loreta.py:
Brain._cleaned = None
method: <bound method Brain._play of <mne.viz._brain._brain.Brain object at 0x7ff68f8975e0>>
method: <bound method Brain.restore_user_scaling of <mne.viz._brain._brain.Brain object at 0x7ff68f8975e0>>

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

One possibility is that the Brain is never properly/completely __init__ed, because I had to add the getattr for _cleaned, but this is set in __init__ I think.

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

(and then the example fails, and during cleanup it does the GC check, which fails because cleanup was never attempted)

@GuillaumeFavelier
Copy link
Contributor

GuillaumeFavelier commented Apr 8, 2022

This happens because we use partial in mne/viz/backends/_qt.py. I chose partial to patch the NotImplementedError in Brain.__hash__() (for PySide2), I'll have to find something else.

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

I chose partial to patch the NotImplementedError in Brain.hash(), I'll have to find something else.

It would probably work to just have Brain.__hash__ return id(self). As long as we really do clean everything up so Qt doesn't try to call the method again, it hopefully wouldn't lead to segfaults :)

@GuillaumeFavelier
Copy link
Contributor

It would probably work to just have Brain.__hash__ return id(self)

If you do not mind, I can try in this PR.

it hopefully wouldn't lead to segfaults

If it is the case, it is a sign that cleaning is not thorough anyway

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

... actually, even better would be to just set self._hash = time.time_ns() during __init__.py, and have __hash__ return this value. There is no way in practice that we will instantiate two brain objects at the same time, right? :)

But really I don't think it's going to matter much, we just have to find a way to placate pyside2. Do you know offhand, does it hash the self of any bound method for some reason? I searched a tiny bit and didn't find anything.

If you do not mind, I can try in this PR.

Yes, please test locally then push here, and I'll merge if CIs are green.

@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

... please push with [circle front] or touch an example that uses Brain, though!

@larsoner larsoner merged commit 41d38b7 into mne-tools:main Apr 8, 2022
@larsoner
Copy link
Member Author

larsoner commented Apr 8, 2022

Thanks for the quick fix @GuillaumeFavelier !

@larsoner larsoner deleted the circle2 branch April 8, 2022 16:11
larsoner added a commit to wmvanvliet/mne-python that referenced this pull request Apr 19, 2022
* upstream/main: (40 commits)
  FIX: Flake (mne-tools#10540)
  FIX: Correct link (mne-tools#10536)
  DOC: Update installers (mne-tools#10535)
  ENH: Add dark mode to website (mne-tools#10523)
  WIP: Copy BEM surfaces by default (don't symlink) (mne-tools#10531)
  Avoid lowpass=0 in brainvision data (mne-tools#10517)
  DOC: Update installers [skip azp] [skip actions] (mne-tools#10528)
  FIX: Fix for old build (mne-tools#10527)
  Fix line noise at wrong frequencies (mne-tools#10525)
  FIX : read fids in eeglab (mne-tools#10521)
  MAINT: Prefer PySide6 in testing (mne-tools#10513)
  ENH: Add overview_mode support (mne-tools#10501)
  MRG: Updates for qtpy in mne-qt-browser (mne-tools#10509)
  BUG: Fix bug with themes on macOS (mne-tools#10500)
  MAINT: Bump installer links (mne-tools#10511)
  Add metadata to combine_channels (mne-tools#10504)
  MAINT: Standardize tests (mne-tools#10502)
  CI: Test circle (mne-tools#10506)
  ENH: Use HiDPI splash screen on HiDPI screens (mne-tools#10503)
  WIP,MNT: Add support for QtPy (mne-tools#10430)
  ...
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.

2 participants