Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve plotting quality #2301

Open
germa89 opened this issue Sep 4, 2023 · 8 comments
Open

Improve plotting quality #2301

germa89 opened this issue Sep 4, 2023 · 8 comments
Assignees

Comments

@germa89
Copy link
Collaborator

germa89 commented Sep 4, 2023

Checking the pytest-pyvista extension, I have realised that the plotting quality is not great (at least on the CICD).

Plots for demonstration:

On the left, the ones generated in my laptop (MacOS X, Python 3.10, MAPDL v22.2).
On the right, the ones generated inside a MAPDL Ubuntun container with (Ubuntu, Python 3.10, MAPDL v22.2).

image image image image
@germa89
Copy link
Collaborator Author

germa89 commented Sep 4, 2023

Pinging @ansys/pyansys-core team for feedback. Especially @AlejandroFernandezLuces

@AlejandroFernandezLuces
Copy link
Contributor

This problem, as well as all of the problems we've seen with PyVista screenshots in PyMAPDL lead me to think that there is a problem with how PyVista is getting the screenshots when the plotter is not in visual mode.

I've already reviewed the scraper for PR #2046, which was potentially where problems like this could arise, and everything is looking good there at the moment. The only problematic part left is the PyVista plotter. PyMAPDL is using PyVista fine, so there shouldn't be anything wrong here regarding this.

We will need assistance from PyVista people to fix these errors. As soon as we get in touch with them, I'll arise this issue with them.

@germa89
Copy link
Collaborator Author

germa89 commented Sep 15, 2023

@AlejandroFernandezLuces is there a way to bypass all the fails in pytest??

@AlejandroFernandezLuces
Copy link
Contributor

You want to temporary mark those test to be skipped because they will fail, while it doesn't get fixed right? You can skip specific tests if you need it with this decorator:

https://docs.pytest.org/en/7.1.x/how-to/skipping.html#skipping-test-functions

Also, you can skip tests based in a condition too, for example:

https://github.com/pyvista/pyvista/blob/9c53f64238c2adc097c0f145dd0facee19d3ff3d/tests/plotting/test_actor.py#L10-L12

Something like this should work:

skip_pyvista = pytest.mark.skip(reason="PyVista is not getting the right screenshots")

@skip_pyvista
def test_yourtest():
   ...

@germa89
Copy link
Collaborator Author

germa89 commented Oct 16, 2023

@AlejandroFernandezLuces

I recently implemented #2359 which basically recache the images in the PR when they don't match the ones in the repo. This is quite useful given how unstable is pyvista backend.

You can can see an example here: a1c5b74

I hope this example can help us to identify issues with the backend.

@germa89
Copy link
Collaborator Author

germa89 commented Dec 11, 2023

@AlejandroFernandezLuces is it worthy to ping Pyvista mantainers? (Bane, Alex, Tetsuo)

@AlejandroFernandezLuces
Copy link
Contributor

It might be worth it. Let me double check after the hackathon this issue and I'll let you know

@AlejandroFernandezLuces
Copy link
Contributor

Update on this:

@github-actions github-actions bot removed the Future label Jun 28, 2024
@germa89 germa89 mentioned this issue Jul 12, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants