MRG: Refactor OFF_SCREEN variable#7276
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7276 +/- ##
==========================================
+ Coverage 87.06% 89.79% +2.73%
==========================================
Files 447 447
Lines 79824 80610 +786
Branches 12880 12882 +2
==========================================
+ Hits 69496 72387 +2891
+ Misses 7512 5390 -2122
- Partials 2816 2833 +17 |
|
What do you think @larsoner , @agramfort ? |
|
@larsoner let us know what you think. I don't fully grasp the consequences of this but if it works and CIs are green it cannot be too bad :) |
|
Actually it's fine to set it globally but we just have to remember to set the original value. My previous changes brought confusion/redundancy of two variables doing the same thing. |
|
Makes me think that we can use a |
|
I trust your judgement here.
… |
|
I would say it's good enough now. We can always make it nicer later. |
|
@larsoner merge if you're happy |
larsoner
left a comment
There was a problem hiding this comment.
Indeed these should be contextmanagers so that we ensure that the state is restored even in the case of an error
|
(but we can do that in another PR, since it's not a problem with these changes specifically) |
|
Thanks @GuillaumeFavelier |
* Refactor OFF_SCREEN variable * Dont forget to restore original value
* Refactor OFF_SCREEN variable * Dont forget to restore original value
This PR prevents setting
pyvista.OFF_SCREENglobally (i.e. in tests, ... etc) by managing a rendererMNE_3D_OFF_SCREENvariable instead.Also, before #7266, the tests were done offscreen by default with
_use_test_3d_backend()so this PR fixes this.This variable is still set globally in
_testing_context()(this one restores the original value) and in thedocwhich is fine by me.