Fix matplotlib backend setting for test suite

[adeak]

This is to complement #2823.

The backend choice issue got me thinking. We only seem to be setting the backend for tests in one place:

pyvista/tests/conftest.py

Lines 11 to 19 in 5d6101b

	@fixture(scope='session')
	def set_mpl():
	"""Avoid matplotlib windows popping up."""
	try:
	import matplotlib
	except Exception:
	pass
	else:
	matplotlib.use('agg', force=True)

However, looking at the pytest docs there's no explicit mention of session-scoped fixtures also being autouse (in fact for the example module-scoped fixture the opposite is true: example tests request it).

If I add a dummy test that does

def test_mpl_backend():
    import matplotlib
    assert matplotlib.get_backend() == 'agg'

this promptly fails with pytest -v -k test_mpl_backend, and changing the fixture to autouse makes this test pass. I think this is what's missing.

I wondered if I should actually add this test to the test suite, but it would be sort of a meta-test, testing our test setup rather than the library code. So I've left it out for now, let me know if you think otherwise.

[codecov]

Codecov Report

Merging #2828 (e9aad92) into main (5d6101b) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2828      +/-   ##
==========================================
+ Coverage   94.00%   94.02%   +0.02%     
==========================================
  Files          76       76              
  Lines       16390    16405      +15     
==========================================
+ Hits        15407    15425      +18     
+ Misses        983      980       -3     

[akaszynski]

Nice work! We might not even need to add in much of the logic in #2823. We can just check if matplotlib is available. Then again, it's a bit more robust at the cost of complexity. Let me know what you think.

---

I wondered if I should actually add this test to the test suite, but it would be sort of a meta-test, testing our test setup rather than the library code. So I've left it out for now, let me know if you think otherwise.

I think it's a good idea to add it in. This will stop us from changing it and seeing our tests fail (intermittently).

[adeak]

Nice work! We might not even need to add in much of the logic in #2823. We can just check if matplotlib is available. Then again, it's a bit more robust at the cost of complexity. Let me know what you think.

I think it's good to have, especially since it doesn't impact many tests. It's good to have an independent safety net to protect against things blowing up in CI. Even if we don't expect it to be necessary. Question is whether it's too much of a maintenance burden going forward.

I wondered if I should actually add this test to the test suite, but it would be sort of a meta-test, testing our test setup rather than the library code. So I've left it out for now, let me know if you think otherwise.

I think it's a good idea to add it in. This will stop us from changing it and seeing our tests fail (intermittently).

OK, thanks, I've added it. Created a new file called test_meta.py for future addition of other tests that don't test library code. I considered adding test_tinypages() here, but that seems convoluted enough that it's better to keep it standalone.

[akaszynski]

Question is whether it's too much of a maintenance burden going forward.

It should be minimal.

[akaszynski]

Are we going to require the usage of matplotlib for our test suite? #2823 makes it optional, which means this metatest will fail immediately without matplotlib.

[adeak]

Are we going to require the usage of matplotlib for our test suite? #2823 makes it optional, which means this metatest will fail immediately without matplotlib.

Good catch, thanks. This test should only fail if matplotlib is otherwise available. Let me fix that.

[adeak]

So this is interesting: VTK seems to depend on matplotlib:

$ python3 -mvenv tmpenv
$ . tmpenv/bin/activate
(tmpenv) $ pip install vtk
[snip]
(tmpenv) $ pip freeze
aiohttp==3.8.1
aiosignal==1.2.0
async-timeout==4.0.2
attrs==21.4.0
charset-normalizer==2.1.0
cycler==0.11.0
fonttools==4.33.3
frozenlist==1.3.0
idna==3.3
kiwisolver==1.4.3
matplotlib==3.5.2
multidict==6.0.2
numpy==1.22.4
packaging==21.3
Pillow==9.1.1
pyparsing==3.0.9
python-dateutil==2.8.2
six==1.16.0
vtk==9.1.0
wslink==1.6.5
yarl==1.7.2

I will still make the test optional, but it all might be a moot point. Unless we change the backend from VTK to something else ;)

[adeak]

According to johnnydep this is the dependency tree of the vtk package:

name                                      summary
----------------------------------------  -------------------------------------------------------------------------------------------------------
vtk                                       VTK is an open-source toolkit for 3D computer graphics, image processing, and visualization
├── matplotlib>=2.0.0                     Python plotting package
│   ├── cycler>=0.10                      Composable style cycles
│   ├── fonttools>=4.22.0                 Tools to manipulate font files
│   ├── kiwisolver>=1.0.1                 A fast implementation of the Cassowary constraint solver
│   ├── numpy>=1.17                       NumPy is the fundamental package for array computing with Python.
│   ├── packaging>=20.0                   Core utilities for Python packages
│   │   └── pyparsing!=3.0.5,>=2.0.2      pyparsing module - Classes and methods to define and execute parsing grammars
│   ├── pillow>=6.2.0                     Python Imaging Library (Fork)
│   ├── pyparsing>=2.2.1                  pyparsing module - Classes and methods to define and execute parsing grammars
│   └── python-dateutil>=2.7              Extensions to the standard Python datetime module
│       └── six>=1.5                      Python 2 and 3 compatibility utilities
└── wslink>=1.0.4                         Python/JavaScript library for communicating over WebSocket
    └── aiohttp                           Async http client/server framework (asyncio)
        ├── aiosignal>=1.1.2              aiosignal: a list of registered asynchronous callbacks
        │   └── frozenlist>=1.1.0         A list-like structure which implements collections.abc.MutableSequence
        ├── async-timeout<5.0,>=4.0.0a3   Timeout context manager for asyncio programs
        ├── attrs>=17.3.0                 Classes Without Boilerplate
        ├── charset-normalizer<3.0,>=2.0  The Real First Universal Charset Detector. Open, modern and actively maintained alternative to Chardet.
        ├── frozenlist>=1.1.1             A list-like structure which implements collections.abc.MutableSequence
        ├── multidict<7.0,>=4.5           multidict implementation
        └── yarl<2.0,>=1.0                Yet another URL library
            ├── idna>=2.0                 Internationalized Domain Names in Applications (IDNA)
            └── multidict>=4.0            multidict implementation

[akaszynski]

Regardless, we need this PR:
https://github.com/pyvista/pyvista/runs/6990657767?check_suite_focus=true

Thanks for looking into this. Didn't realize VTK depended on wslink too!