MNT: BackgroundPlotter has no attribute named image

[GuillaumeFavelier]

This PR follows #9313 (comment). For now, I'm trying to understand what is going on exactly.

Closes #9007

[larsoner]

I'm not sure [circle front] will catch the failures as it's only in some examples, not all. And looking at the logs I think it's about to succeed in building. It might be worth modifying just one failing example and pushing with [skip azp] [skip github] and letting CircleCI build just the one example. Take your pick from this list:

https://app.circleci.com/pipelines/github/mne-tools/mne-python/7888/workflows/26ebfff7-4b79-426c-9c8a-786e954ce2e6/jobs/27434

[GuillaumeFavelier]

From a quick look at the Check installation step, I saw nothing irregular.

I'll investigate locally (on TTY with xvfb).

[larsoner]

In the meantime you could push a commit here, that way if it fails here but not locally you can "Rebuild with SSH". Also it would be good to see it fail on some commit here so we know that, if you push a fix, it actually fixes the problem

[GuillaumeFavelier]

Is it just me or no one of those failling examples actually uses BackgroundPlotter? 👀

[GuillaumeFavelier]

Nevermind, I think I found one

[larsoner]

It passed, how annoying!

[larsoner]

Looking through the actual build log:

generating gallery for auto_examples/io... [100%] plot_read_noise_covariance_matrix.py
2991.2 s : 1.84 GB    

generating gallery for auto_examples/simulation... [ 25%] plot_simulate_evoked_data.py
2996.2 s : 1.84 GB    
WARNING: /home/circleci/project/examples/simulation/plot_simulate_evoked_data.py failed to execute correctly: Traceback (most recent call last):
  File "/home/circleci/.local/lib/python3.8/site-packages/sphinx_gallery/scrapers.py", line 333, in save_figures
    rst = scraper(block, block_vars, gallery_conf)
  File "/home/circleci/.local/lib/python3.8/site-packages/pyvista/utilities/sphinx_gallery.py", line 49, in __call__
    plotter.screenshot(fname)
  File "/home/circleci/.local/lib/python3.8/site-packages/pyvista/plotting/plotting.py", line 3122, in screenshot
    return self._save_image(self.image, filename, return_img)
  File "/home/circleci/.local/lib/python3.8/site-packages/vtkmodules/qt/QVTKRenderWindowInteractor.py", line 335, in __getattr__
    raise AttributeError(self.__class__.__name__ +
AttributeError: BackgroundPlotter has no attribute named image

Maybe try modifying plot_simulate_evoked_data.py? If that caused a problem and prevented _all_plotters from being cleared or whatever, then it would then "fail" in all other following examples in the same way, each time never being cleared.

This is a form of a bug in the PyVista scraper in that, no matter what, it should clear _all_plotters or somehow prevent re-scraping of the same plotter more than once. One way to deal with this is to put the scraping in a try: ... finally: where the finally clause does the close and removal in _all_plotters.

[larsoner]

I can reproduce locally with:

$ PATTERN=simulate_evoked make html_dev-pattern
...
generating gallery for auto_examples/io... [100%] plot_read_noise_covariance_matrix.py                                                                                              
WARNING: /home/larsoner/python/mne-python/examples/simulation/plot_simulate_evoked_data.py failed to execute correctly: Traceback (most recent call last):                          
  File "/home/larsoner/python/sphinx-gallery/sphinx_gallery/scrapers.py", line 333, in save_figures
    rst = scraper(block, block_vars, gallery_conf)
  File "/home/larsoner/python/pyvista/pyvista/utilities/sphinx_gallery.py", line 49, in __call__
    plotter.screenshot(fname)
  File "/home/larsoner/python/pyvista/pyvista/plotting/plotting.py", line 3122, in screenshot
    return self._save_image(self.image, filename, return_img)
  File "/home/larsoner/.local/lib/python3.9/site-packages/vtkmodules/qt/QVTKRenderWindowInteractor.py", line 339, in __getattr__
    raise AttributeError(self.__class__.__name__ +
AttributeError: BackgroundPlotter has no attribute named image

[larsoner]

I think I realized the real problem -- when image does not exist, it raises AttributeError, which causes Python MRO to look at the super class, which is VTKRenderWindowInteractor or whatever. If I change the PyVista AttributeError calls to RuntimeError we get a more useful traceback:

generating gallery for auto_examples/io... [100%] plot_read_noise_covariance_matrix.py                                                                                              
WARNING: /home/larsoner/python/mne-python/examples/simulation/plot_simulate_evoked_data.py failed to execute correctly: Traceback (most recent call last):                          
  File "/home/larsoner/python/sphinx-gallery/sphinx_gallery/scrapers.py", line 333, in save_figures
    rst = scraper(block, block_vars, gallery_conf)
  File "/home/larsoner/python/pyvista/pyvista/utilities/sphinx_gallery.py", line 49, in __call__
    plotter.screenshot(fname)
  File "/home/larsoner/python/pyvista/pyvista/plotting/plotting.py", line 3122, in screenshot
    return self._save_image(self.image, filename, return_img)
  File "/home/larsoner/python/pyvista/pyvista/plotting/plotting.py", line 737, in image
    raise RuntimeError('\nThis plotter has not yet been setup and rendered '
RuntimeError: 
This plotter has not yet been setup and rendered with ``show()``.
Consider setting ``off_screen=True`` for off screen rendering.

This is despite me seeing the window up and fully rendered a second or so before getting the error.

So I would say there are three problems:

1. PyVista should always clear _all_plotters in its scraper
2. It should raise RuntimeError rather than AttributeError in the property so that no superclass name resolution is attempted
3. We need to fix our use of BackgroundPlotter (or PyVista has some bug) where it thinks no rendering has been done even though it has been

[GuillaumeFavelier]

Thank you so much for investigating because it was definitely a weird one.

Indeed I pinned down the issue between image()/screenshot() (the functions raising AttributeError) and the dependency to show()/render() (recently?) introduced. The problem is not with the self.image property but rather self._rendered, this is the so called setup in :

This plotter has not yet been setup...

For the long explanation, BackgroundPlotter does not have such attribute compared to the standard Plotter because the render() function which is supposed to set the correct value for this parameter is actually shadowed by:

    @conditional_decorator(threaded, platform.system() == "Darwin")
    def render(self) -> None:
        """Override the ``render`` method to handle threading issues."""
        return self.render_signal.emit()

and strangely renamed by:

    @wraps(BasePlotter.render)
    def _render(self, *args: Any, **kwargs: Any) -> BasePlotter.render:
        """Wrap ``BasePlotter.render``."""
        return BasePlotter.render(self, *args, **kwargs)

So, I think calling this _render might mitigate. It did for me locally.

[GuillaumeFavelier]

If it works here, the fix should be ported upstream somehow.

[GuillaumeFavelier]

What do you think @larsoner ?

[larsoner]

My guess is that it's supposed to emit this signal so that on the next paintEvent a render actually gets called (?). So another option is probably just to processEvents and it should also take care of the issue. Want to try that?

[GuillaumeFavelier]

Where? There is already one here:

mne-python/mne/viz/backends/_qt.py

Line 510 in a921140

self._process_events()

[larsoner]

Ahh seems like it should have taken care of it. I wonder if this has something to do with the delay between when you call show and when an X window actually gets displayed -- it's asynchronous so it can be 100-200 ms or so, which might be enough that the process_events there ends up being a no-op because the window hasn't shown yet (and there was no paintEvent called, and hence no render).

So maybe the thing to do is to move the for plotter in self._all_plotters: plotter._render() up into the show call instead. That way we only do these extra renders once, during the show event, rather than during every update call.

[GuillaumeFavelier]

Sure, I can move it in show().

[larsoner]

You should be able to check locally with the make html_dev-pattern call to see whether or not it works

[GuillaumeFavelier]

I did PATTERN=simulate_evoked make html_dev-pattern before pushing and it works as well.

[larsoner]

Can you touch that example, the 4D phantom BTi example, and the source alignment example in this PR? Would be good to see which of the problems it fixes

[larsoner]

All fixed!

• https://27532-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/source-modeling/plot_source_alignment.html
• https://27532-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/sample-datasets/plot_phantom_4DBTi.html
• https://27532-1301584-gh.circle-artifacts.com/0/dev/auto_examples/simulation/plot_simulate_evoked_data.html

Will merge once CIs come back happy

[agramfort]

awesome !

…

[larsoner]

Thanks @GuillaumeFavelier !