Use headless ("Agg") matplotlib backend in CI #1133
Conversation
|
We use the same solution in SCT, but there we set it as a pytest setting: @joshuacwnewton and I had a debate about which file was better to set this variable in: spinalcordtoolbox/spinalcordtoolbox#3420 (comment) and he had a good point, so maybe this should be moved there to |
|
Drat, it didn't work: https://github.com/ivadomed/ivadomed/actions/runs/2342071673
I don't get it, that should have worked. |
Pull Request Test Coverage Report for Build 2354062718
💛 - Coveralls |
|
|
||
| gui_env = ['TKAgg', 'GTKAgg', 'Qt4Agg', 'WXAgg'] | ||
| selected_gui_env = [] | ||
| for gui in gui_env: | ||
| try: | ||
| matplotlib.use(gui) | ||
| from matplotlib import pyplot as plt | ||
|
|
||
| selected_gui_env = gui | ||
| break | ||
| except: | ||
| continue | ||
| # If none works | ||
| if selected_gui_env == []: | ||
| from matplotlib import pyplot as plt | ||
|
|
||
| if matplotlib.get_backend() == "agg": |
There was a problem hiding this comment.
I'm pretty sure this loop is redundant: https://matplotlib.org/stable/users/explain/backends.html
Without a backend explicitly set, Matplotlib automatically detects a usable backend based on what is available on your system and on whether a GUI event loop is already running. The first usable backend in the following list is selected: MacOSX, QtAgg, GTK4Agg, Gtk3Agg, TkAgg, WxAgg, Agg.
and calling use() in the middle of tests is messing the tests up by leaking state from one test to another, because use() is setting an internal global variable and hence causing tests to subtly change their behaviour depending on the order they happen to run in.
Especially because because none of the options were 'Agg', that might explain why the tests were being forced to try to load TkAgg even though it was unnecessary and in fact temporarily broken.
kousu
changed the title
|
@joshuacwnewton helped me make it work! |
Force Agg matplotlib backend Fixes #1132
Leave the choice of backend to the user/environment; trying to guess causes more trouble than it saves IMO. Co-authored-by: Joshua Newton <joshuacwnewton@gmail.com>
Do you want to be moved to the top of the file instead of removed? The tests passed without that, do you think it's really necessary? |
The Is it possible that the |
Yup
I guess that'd be necessary if someone runs it as a script ivadomed/ivadomed/scripts/visualize_and_compare_testing_models.py Lines 111 to 115 in bd904e5 Moreover, I believe there are no tests yet for it. |
Oh true, I didn't notice that. I'll put it in then merge :) |
Checklist
GitHub
PR contents
Description
Force Agg matplotlib backend to avoid problems in CI. CI should be headless anyway, there's no need to try the other six backends.
Linked issues
Fixes #1132, which is the most recent symptom, but it's not the only tricky thing that can come up when mixing and matching rendering backends and headless test machines.