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

Adding test_plotting in tests #107

Merged
merged 8 commits into from
Jun 12, 2024

Conversation

FloraSauerbronn
Copy link
Contributor

Creating test_plotting.py with the first test of plot_transect, using pytest-mpl.

Copy link
Member

@ocefpaf ocefpaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need 3 extra steps to activate the image comparison tests here:

  1. add pytest-mpl to the requirements-dev.txt file..
  2. generate the comparison images with pytest --mpl-generate-path=tests/baseline.
  3. add the --mpl option in the line that run the tests in .github/workflows/tests.yml.

You can find more about pytest-mpl here.

from gliderpy.plotting import plot_transect
from gliderpy.fetchers import GliderDataFetcher

@pytest.mark.mpl_image_compare
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to give the test function a directory where to look for the images for the comparison.

Suggested change
@pytest.mark.mpl_image_compare
@pytest.mark.mpl_image_compare(baseline_dir=root.joinpath("baseline/"))


from gliderpy.plotting import plot_transect
from gliderpy.fetchers import GliderDataFetcher

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using figures in tests we do not want matplotlib to fire up its Graphical User Interface (GUI) to avoid holding up the tests with a Window waiting to be closed.

Suggested change
import matplotlib as mpl
mpl.use("Agg")


from gliderpy.plotting import plot_transect
from gliderpy.fetchers import GliderDataFetcher

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this to get the current directory to build the path to the baseline directory, where the images for the comparisons are held.

Suggested change
from pathlib import Path
root = Path(__file__).parent

@ocefpaf
Copy link
Member

ocefpaf commented Jun 11, 2024

@FloraSauerbronn, as you can see the PR is failing as expected when we change the plot and the image deviates from the baseline. We can revert it back to temperature now and things should pass.

@FloraSauerbronn
Copy link
Contributor Author

@FloraSauerbronn, as you can see the PR is failing as expected when we change the plot and the image deviates from the baseline. We can revert it back to temperature now and things should pass.

Sorry @ocefpaf, I forgot to change it back before the push

@ocefpaf
Copy link
Member

ocefpaf commented Jun 11, 2024

The tests are failing now b/c there is enough difference to trigger a real mismatch. The diff image is:

image

That is probably b/c of fonts differences between you installation of matplotlib and the one in the CIs. BTW, what version of matplotlib do you have?

I'll regenerate the image here and push to confirm if that is the case.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 11, 2024

As I suspected, the matplolib on my machine was able to create an image that had a match with the one in the CIs. We could also get this to pass by adjust the tolerance but, in this case, we could solve it without resorting to that.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 11, 2024

pre-commit.ci autofix

@FloraSauerbronn
Copy link
Contributor Author

The tests are failing now b/c there is enough difference to trigger a real mismatch. The diff image is:

image

That is probably b/c of fonts differences between you installation of matplotlib and the one in the CIs. BTW, what version of matplotlib do you have?

I'll regenerate the image here and push to confirm if that is the case.

I'm using
matplotlib 3.9.0
matplotlib-inline 0.1.7

What is the CIs ?

@ocefpaf
Copy link
Member

ocefpaf commented Jun 11, 2024

What is the CIs ?

Continuous Integration. The system we are using to run the tests here.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 12, 2024

I'll merge this to reduce the merge conflicts and we can address the last pre-commit-ci failure in a new PR.

@ocefpaf ocefpaf merged commit 8de0bec into ioos:main Jun 12, 2024
11 of 12 checks passed
@FloraSauerbronn FloraSauerbronn deleted the create-test-transect branch June 27, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants