Skip MathText test for VTK and Matplotlib version incompatibilities

[akaszynski]

It's necessary to conditionally skip a test in tests/plotting/test_plotter.py as it's possible to have a local testing enviornment where this test always fails:

Problem Overview

In tests/plotting/test_plotter.py we have:

pyvista/tests/plotting/test_plotting.py

Line 2501 in 1f78d08

For VTK<=9.2.2, this requires matplotlib<3.6

Yet we don't specify a conditional matplotlib requirement in requirements_test.txt:

pyvista/requirements_test.txt

Line 13 in 1f78d08

matplotlib<3.7.0

This means that the test will fail with the following environment:

$ python3.8 -m venv .venv
$ source .venv/bin/activate
$ pip install pip wheel -U
$ pip install -e .
$ pip install -r requirements_test.txt 
$ pip install vtk==9.1.0
$ pytest tests/plotting/ -k latex
tests/plotting/test_plotting.py F                                             [100%]

===================================== FAILURES ======================================
________________________________ test_add_text_latex ________________________________

    @pytest.mark.skipif(
        not vtk.vtkMathTextFreeTypeTextRenderer().MathTextIsSupported(),
        reason='Math text is not supported.',
    )
    def test_add_text_latex():
        """Test LaTeX symbols.
    
        For VTK<=9.2.2, this requires matplotlib<3.6
        """
        plotter = pyvista.Plotter()
        plotter.add_text(r'$\rho$', position='upper_left', font_size=150, color='blu)
>       plotter.show()

tests/plotting/test_plotting.py:2504: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pyvista/plotting/plotting.py:6413: in show
    self.close()
pyvista/plotting/plotting.py:4200: in close
    self._before_close_callback(self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pytest_pyvista.pytest_pyvista.VerifyImageCache object at 0x7f4584573eb0>
plotter = <pyvista.plotting.plotting.Plotter object at 0x7f4584634430>

    def __call__(self, plotter):
        """Either store or validate an image.
    
        Parameters
        ----------
        plotter : pyvista.Plotter
            The Plotter object that is being closed.
    
        """
        if self.skip:
            return
    
        if not VTK9:
            if self.skip_image_cache_vtk8:
                return
            else:
                raise RuntimeError("Image cache is only valid for VTK9+")
    
        if self.ignore_image_cache:
            return
    
        if self.n_calls > 0:
            test_name = f"{self.test_name}_{self.n_calls}"
        else:
            test_name = self.test_name
        self.n_calls += 1
    
        if self.high_variance_test:
            allowed_error = self.var_error_value
            allowed_warning = self.var_warning_value
        else:
            allowed_error = self.error_value
            allowed_warning = self.warning_value
    
        # some tests fail when on Windows with OSMesa
        if os.name == "nt" and self.windows_skip_image_cache:
            return
        # high variation for MacOS
        if platform.system() == "Darwin" and self.macos_skip_image_cache:
            return
    
        # cached image name. We remove the first 5 characters of the function name
        # "test_" to get the name for the image.
        image_filename = os.path.join(self.cache_dir, test_name[5:] + ".png")
    
        if not os.path.isfile(image_filename) and self.fail_extra_image_cache:
            raise RuntimeError(f"{image_filename} does not exist in image cache")
        # simply save the last screenshot if it doesn't exist or the cache
        # is being reset.
        if self.reset_image_cache or not os.path.isfile(image_filename):
            return plotter.screenshot(image_filename)
    
        # otherwise, compare with the existing cached image
        error = pyvista.compare_images(image_filename, plotter)
        if error > allowed_error:
>           raise RuntimeError(
                f"{test_name} Exceeded image regression error of "
                f"{allowed_error} with an image error equal to: {error}"
            )
E           RuntimeError: test_add_text_latex Exceeded image regression error of 500 
with an image error equal to: 6860.6836601301275                                    

.venv/lib/python3.8/site-packages/pytest_pyvista/pytest_pyvista.py:165: RuntimeError
------------------------------- Captured stderr call --------------------------------
2023-01-17 11:08:41.288 (   1.193s) [        AB69E740]vtkMatplotlibMathTextUt:984    
ERR| vtkMatplotlibMathTextUtilities (0x33f4b50): MaskParser is not initialized!     
... <redacted, line repeats several times>
--------------------------------- Captured log call ---------------------------------
ERROR    root:errors.py:116 MaskParser is not initialized!
... <redacted, line repeats several times>
=========================== memory consumption estimates ============================
tests/plotting/test_plotting.py::test_add_text_latex  - 12.0 MB
============================== short test summary info ==============================
FAILED tests/plotting/test_plotting.py::test_add_text_latex - RuntimeError: test_add_
text_latex Exceeded image regression error of 500 with an...                        
========================= 1 failed, 341 deselected in 1.32s =========================

[codecov]

Codecov Report

Merging #3838 (0cbcfb9) into main (9043cd1) will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3838      +/-   ##
==========================================
+ Coverage   94.11%   94.18%   +0.07%     
==========================================
  Files          87       87              
  Lines       19108    19156      +48     
==========================================
+ Hits        17984    18043      +59     
+ Misses       1124     1113      -11     

[banesullivan]

I disagree with this change. (and it would need to be 9.2.5, not 9.2.2)

Indeed, the MPL version needs to be limited depending on the VTK version (matplotlib<3.6 for VTK<=9.2.2). This is currently handled on CI and not in our requirements_*.txt files. Reference:

pyvista/.github/workflows/testing-and-deployment.yml

Line 183 in 9043cd1

- name: Limit Matplotlib for VTK<=9.2.2

In my opinion, this should be left as is, and we should better document that "For VTK<=9.2.2, this requires matplotlib<3.6" which developers will have to handle locally when using VTK<=9.2.2.

However, I recognize that this opinion introduces confusion and developer burden. So how about this?:

We could modify this test to have the following condition that checks to make sure VTK and MPL are compatible:

@pytest.mark.skipif(
    matplotlib._parse_to_version_info(matplotlib._get_version()) >= (3, 6)
    and pyvista.vtk_version_info <= (9, 2, 2),
    reason='VTK and Matplotlib version incompatibility. For VTK<=9.2.2, MathText requires matplotlib<3.6',
)

[banesullivan]

Though using a private API from MPL might not be the best choice...

[adeak]

Why not just do something along the lines of tuple(map(int, matplotlib.__version__.split('.')[:2]))? Sure, looks messier, but __version__ is very fundamental.

[akaszynski]

Implemented both suggestions in 8638c86.