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

Add @pytest.mark.mpl_image_compare() fixture to test_plottingfuncs.py::test_plot_and_save_no_axes_no_colorbar #806

Closed
ns-rse opened this issue Feb 27, 2024 · 0 comments · Fixed by #810
Labels
Plotting Issues pertaining to the plotting class testing

Comments

@ns-rse
Copy link
Collaborator

ns-rse commented Feb 27, 2024

Noticed this test doesn't compare an image, only the sum of the elements and its shape after reading it back in which I'm unsure whether it demonstrates that the image has been saved without axes or colorbar. Would therefore be useful to add a fixture that uses the pytest-matplotlib extension to save and compare images as is done with others.

@pytest.mark.mpl_image_compare(baseline_dir="resources/img/")
def test_plot_and_save_no_axes_no_colorbar(load_scan_data: LoadScans, plotting_config: dict, tmp_path: Path) -> None:
    """Test plotting without axes and without the colourbar."""
    plotting_config["axes"] = False
    plotting_config["colorbar"] = False
    fig, _ = Images(
        data=load_scan_data.image,
        output_dir=tmp_path,
        filename="01-raw_heightmap",
        title="Raw Height",
        **plotting_config,
    ).plot_and_save()
    return fig
@ns-rse ns-rse added testing Plotting Issues pertaining to the plotting class labels Feb 27, 2024
ns-rse added a commit that referenced this issue Mar 4, 2024
Closes #806

Whislt #806 was meant to address why no Matplotlib image was generated and tested/compared to a target image when
plotting without axes or colorbar I discovered this was because the image is saved with `Images.save_array_figure()`.

This in turn uses
[`matplotlib.pyplot.imsave()`](https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.imsave.html) to save the
image and the test was using
[`skimage.io.imread()`](https://scikit-image.org/docs/stable/api/skimage.io.html#skimage.io.imread) to read the image
back as an array and test the `np.sum()` and `img.shape`.

We no longer need to save arrays nor images as readable arrays (see #804 / #802) and so the need to save the image in
this manner seemed redudant. The `Images.save_figure()` already has logic which excludes the axes and scale bars (see
lines 321 of current changeset).

Further the logic for deciding what to save within `Image.plot_and_save()` seemed overly complicated and if
`Images.save_array_figure()` were being called `Images.plot_and_save()` did not return a `fig` (Matplotlib Image) that
could be saved and tested using `pytest-mpl` extension.

To this end I have...

1. Removed `Images.save_array_figure()`.
2. Tweaked the plotting options under `Images.savefig()` starting at line 321 to use a [tight
   layout](https://matplotlib.org/2.0.2/users/tight_layout_guide.html) which ensures there is no border (see note
   below).
3. Simplified the logic in `Images.plot_and_save()` controlling whether images are saved so that `ValueError` are raised
   if `Images.savefig == False` or `if Images.image_set in ["all", "core"] or self.core_set:` evaluates to
   `False`. Ensures images are explicitly closed to reduce memory usage.
4. Added tests for the raising of `ValueError` exceptions in `Images.plot_and_save()`
5. Tidied up some test names to be consistent (resulted in image name change).
6. Tidied up passing of `plotting_config` into these tests.
7. Added a `pytest-mpl` test for `test_plot_and_save_no_axes_no_colorbar()`, adding the target image as required by #806

Note on Borders

The image generated by `pytest-mpl` against which comparisons are made _does_ have a border because it is saving the
returned `fig` object itself. The actual generated image doesn't have a border (manual checks have been made).
ns-rse added a commit that referenced this issue Mar 4, 2024
In #806 and refactoring (see 07db718) I found an unintended side-effect was that some of the target
directories (specifically `filter_out_path` and `grain_out_path`) were no longer present and the
tests (`tests/test_processing.py`) failed.

These are meant to be explicitly created by `processing.get_out_paths()` but for some reason were now no longer.

For filters the solution seemed counter-intuitive as the images are saved to `filter_out_path` if the image is part of
the `core_set`, otherwise they are saved to `core_out_path`.

For grains the `direction` is appended to the `grain_out_path` once (rather than three times) and the `.mkdir(parents =
True, exists_ok=True)` pathlib method is used to ensure the directory exists prior to attempting to plot.
ns-rse added a commit that referenced this issue Mar 4, 2024
In #806 and refactoring (see 07db718) I found an unintended side-effect was that some of the target
directories (specifically `filter_out_path` and `grain_out_path`) were no longer present and the
tests (`tests/test_processing.py`) failed.

These are meant to be explicitly created by `processing.get_out_paths()` but for some reason were now no longer.

For filters the solution was to ensure the `filter_out_path` is created _and_ use either the `core_out_path` or
`filter_out_path` depending on which image is being saved.

For grains the `direction` is appended to the `grain_out_path` once (rather than three times) and the `.mkdir(parents =
True, exists_ok=True)` pathlib method is used to ensure the directory exists prior to attempting to plot.

In simplifying the logic I found that some of the processing tests failed, for some reason images that were meant to be
part of the `all` set were being plotted by mistake and the tests were looking for these images in the `caplog`. These
have been updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Plotting Issues pertaining to the plotting class testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant