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

Plotting tests fail because of different image sizes #192

Open
lsoucasse opened this issue Sep 26, 2024 · 4 comments
Open

Plotting tests fail because of different image sizes #192

lsoucasse opened this issue Sep 26, 2024 · 4 comments
Labels
bug Something isn't working help wanted Extra attention is needed JOSS publication: PROTEUS TBD before PROTEUS JOSS publication Priority 3: standard Priority level 3: medium time criticality or importance software Relating to software and implementation

Comments

@lsoucasse
Copy link
Member

When comparing reference plots with CI plots, the image sizes are slightly different.

I tried to resize the image before using the compare_image function of matplotlib in tests/integration/test_dummy_integration.py, but the resizing creates small shifts in axes and labels which make the comparison not relevant.

@lsoucasse lsoucasse added bug Something isn't working help wanted Extra attention is needed labels Sep 26, 2024
@lsoucasse lsoucasse changed the title Plotting tests fail becaus of different image sizes Plotting tests fail because of different image sizes Sep 26, 2024
@lsoucasse lsoucasse added the software Relating to software and implementation label Nov 11, 2024
@timlichtenberg
Copy link
Collaborator

It seems these reference plots are quite sensitive to minor changes in various things. Do we think it is a long-lasting solution to directly compare reference plots to one another or should we systematically resort to comparing only output numbers?

@timlichtenberg timlichtenberg added the Priority 3: standard Priority level 3: medium time criticality or importance label Nov 19, 2024
@lsoucasse
Copy link
Member Author

It seems these reference plots are quite sensitive to minor changes in various things. Do we think it is a long-lasting solution to directly compare reference plots to one another or should we systematically resort to comparing only output numbers?

I quite like the idea to test the plots but I don't think it should be systematic for all tests as it involves managing many files.

@stefsmeets
Copy link
Contributor

stefsmeets commented Nov 28, 2024

It seems these reference plots are quite sensitive to minor changes in various things. Do we think it is a long-lasting solution to directly compare reference plots to one another or should we systematically resort to comparing only output numbers?

It's a good question. Comparing numbers is great, and testing the plots does not replace this. However, a visual comparison of the data can often help quickly make sense of the data.

In addition, we want to regression test the plots (a large part of the output from PROTEUS is in the plots after all) as well as improve code coverage (see #268).

About this being a long-lasting solution: the pattern itself is used by matplotlib for their internal testing. I have used the pattern in several projects, and I have not encountered issues, the plots are typically reproducible cross-platform and cross-version.

We should look into the difference however. The major difference between other implementations of this pattern is that they directly work with the fig object, stripping any styling before doing the image compare. This reduces issues with e.g. font rendering (usually the biggest source of differences). Whereas in proteus we let the integration test write all the files first (including styling).

@stefsmeets
Copy link
Contributor

The current code only supports saving the file and returning the figure (e.g. for use in a notebook). We should split out the actual plotting bit from the io (e.g. saving vs. displaying). This will also help solve the issue with the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed JOSS publication: PROTEUS TBD before PROTEUS JOSS publication Priority 3: standard Priority level 3: medium time criticality or importance software Relating to software and implementation
Projects
Status: JOSS Publication
Development

No branches or pull requests

3 participants