Add a sample test using pytest-pyvista

[tkoyama010]

🚀 Pull Request

Add a sample test using pytest-pyvista.

Description

---

Once this sample has been discussed and approved, we will add other tests in subsequent PullRequests.

Reference: pytest-pyvista document (https://pytest.pyvista.org/)

Edit: The best way to try this test is to let the test draw a different figure and see if that results in an error.

[MatthewFlamm]

I recommend using --fail_extra_image_cache option on the CI.

[tkoyama010]

Thank you.
And I felt a little pain about pytest-pyvista not having a repository in conda-forge.

[tkoyama010]

Since it seems that conda-forge can also be created by non-authors, I will consider creating it.
https://github.com/conda-forge/staged-recipes

[tkoyama010]

@bjlittle Could you please tell me how to register a new package in the lock file so that it will be installed by CI when we add a new package via pull request?

[tkoyama010]

Of course, if you don't want to reject this PR LOL.

[bjlittle]

@bjlittle Could you please tell me how to register a new package in the lock file so that it will be installed by CI when we add a new package via pull request?

Ahhh it's a shame that pytest-pyvista isn't on conda-forge, I can help make that happen, if you needed help. From my perspective that would be ideal.

Essentially, the GHA CI ci-locks task renders all the package dependencies each week (scheduled as a GHA cron), but under the hood it's using conda-lock to do this ... the thing I'll need to investigate is whether it honours the use of pip within the YAML, and I don't think it does ATM.

Hence, having pytest-pyvista as a conda package would be ideal ... I'll make time this weekend to confirm this or not for you @tkoyama010

[bjlittle]

Of course, if you don't want to reject this PR LOL.

Of course not ... I'm excited that you're pushing this forwards @tkoyama010 and thanks @MatthewFlamm for getting involved 🍻

[tkoyama010]

I can help make that happen, if you needed help.

@bjlittle
Thanks. I have not created it yet and will give it a try.
Is this the right place to make our submit?
https://github.com/conda-forge/staged-recipes/tree/main

[tkoyama010]

I will update the conda environment after conda-forge/staged-recipes#24058 is released.

[bjlittle]

@tkoyama010 Of course, once pytest-pyvista is available on conda-forge, then we commit to embracing image testing of the geovista examples, due to the open issue #447 both geovista and pyvista will have failing tests.

This puts pressure on resolving #447.

Do you have a feel for whether this is possible soon?

If not, then as a temporary measure I guess I'll have to introduce a pyvista<=0.42.2 pin in the dependencies.

In fact, thinking about this I'm kinda forced to do this now for safety, and push a post release for 0.4.0.

What do you think?

[bjlittle]

@tkoyama010 Of course, once pytest-pyvista is available on conda-forge, then we commit to embracing image testing of the geovista examples, for to #447 both geovista and pyvista will have failing tests.

This puts pressure on resolving #447.

Do you have a feel for whether this is possible soon?

If not, then as a temporary measure I guess I'll have to introduce a pyvista<=0.42.2 pin in the dependencies.

In fact, thinking about this I'm kinda forced to do this now for safety, and push a post release for 0.4.0

[bjlittle]

@tkoyama010 You'll also need to add pytest-pyvista to the pypi dependencies in https://github.com/bjlittle/geovista/blob/main/requirements/pypi-optional-test.txt

[tkoyama010]

pre-commit.ci autofix

[tkoyama010]

pre-commit.ci autofix

[tkoyama010]

I can't resolve the core dumped error, so I'm going to discontinue the investigation once.
Edit: Sorry it is InvocationError it seems that the test code is not recognized by pytest.

[MatthewFlamm]

Are you using osmesa vtk library or using xvfb?

[MatthewFlamm]

It would be useful to add in an environment state and/or a pv.Report() into the CI output before the tests are run.

[bjlittle]

I can't resolve the core dumped error, so I'm going to discontinue the investigation once.

Note that, the CI is using the QT bindings to vtk. I've never attempted to render a plot on a GH runner, does it have to be the OSMESA build of vtk instead?

For example, see https://github.com/bjlittle/geovista/blob/main/requirements/locks/py311-lock-linux-64.txt

[bjlittle]

Are you using osmesa vtk library or using xvfb?

Neither.

Is it the case that we should have:

• osmesa vtk or
• Qt vtk and xvfb

It's only Qt vtk at present.

[MatthewFlamm]

I think these are the options that PyVista has used in the recent past. I remember xvfb was flaky in the CI. The way pytest-pyvista works is that it hooks into the before_close_callback machinery on the Plotter, so if you run with normal vtk bindings it will try to plot an interactive session and wait for it to close. I never wrapped my head around why xvfb works in this case.

I could envision adding functionality to pytest-pyvista to work without this requirement by only using Plotter.screenshot. In this way it would be slightly easier to use pytest-mpl too. But then you wouldn't be able to directly test your examples with images as they all use Plotter.show.

[tkoyama010]

pre-commit.ci autofix

[MatthewFlamm]

Since this PR seems to be the preferred one to move forward, I will place here. The other PRs have regression errors. I strongly recommend using a pyvista theme that is set in a conftest.py to avoid differences in themes for different contributors. pyvista's is here (although I'd prefer it to be in a fixture that is autouse=True) to make sure it is reset before every test.

https://github.com/pyvista/pyvista/blob/19e7b15b4ba8a3466926804a6acab66b35377da9/tests/plotting/conftest.py#L13

https://github.com/pyvista/pyvista/blob/19e7b15b4ba8a3466926804a6acab66b35377da9/pyvista/plotting/themes.py#L3076-L3094

Edit: It looks like the image resolution in the file is much higher than image resolution used in pyvista. I think a higher image resolution would require a higher allowable threshold based on the way it is defined currently. You might also find that a lower resolution might be more robust to having pixel shift differences.

[tkoyama010]

Thanks. You are best.

[bjlittle]

Closed by #470.

Thanks so much for pushing this forwards @tkoyama010 🍻