Skip to content

Commit

Permalink
fix(plotting): remove dilation if images are large
Browse files Browse the repository at this point in the history
The section which automatically dilated masks had been commented out as part of the refactoring for
`better-tracing`. This meant that the related test (`tests/test_plottingfuncs.py::test_mask_dilation()`) failed.

After discussion it was decided to remove this functionality completely. We have left the
`topostats.plottingfuncs.dilate_binary_image()` which may be used elsewhere. It isn't currently but there _is_ an
argument/option in `default_config.yaml` under `mask_smoothing_params` which indicates the dilation is undertaken during
smoothing so perhaps we can simplify the code by calling the `dilate_binary_image()` function we have in place with
tests and reduce the amount of code within the smoothing module.

Further the step which turns a mask with numbered layers to binary (`0`/`1`) has been removed because plots now require
there to be different layers of masking to differentiate individual molecules.

Another issue is that the tests were developed with a value of `zrange: [null, null]` but in this branch the
`default_config.yaml` has been modified to `zrange: [-2, 6]` which results in six tests failing. The
`default_config.yaml` has been reverted to `zrange: [null, null]`.

This does raise a couple of issues.

1. What should the defaults be in the `main` branch and in releases?

When developing features and testing new code it may be desirable to use custom configurations but what should we be
using as defaults for releases? What set of values works best for _most_ use cases?

2. Use `--config <custom_config>.yaml`

Directly related to this is that we should not be changing values in `default_config.yaml` when undertaking development.
Rather we should use the `--config` flag with a custom configuration file. When new options need adding that isn't a
problem to add them to `default_config.yaml` but when modifying values it is important to ensure all tests pass.

It can take some time to track down and work out the root cause of these test failures!
  • Loading branch information
ns-rse committed Sep 24, 2024
1 parent 6cbc79f commit af7a2d9
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 26 deletions.
17 changes: 0 additions & 17 deletions tests/test_plottingfuncs.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ def test_load_mplstyle(style: str, axes_titlesize: int, font_size: float, image_
def test_dilate_binary_image(binary_image: np.ndarray, dilation_iterations: int, expected: np.ndarray) -> None:
"""Test the dilate binary images function of plottingfuncs.py."""
result = dilate_binary_image(binary_image=binary_image, dilation_iterations=dilation_iterations)

np.testing.assert_array_equal(result, expected)


Expand Down Expand Up @@ -277,22 +276,6 @@ def test_high_dpi(minicircle_grain_gaussian_filter: Grains, plotting_config: dic
return fig


@pytest.mark.mpl_image_compare(baseline_dir="resources/img/")
def test_mask_dilation(plotting_config: dict, tmp_path: Path) -> None:
"""Test the plotting of a mask with a different colourmap (blu)."""
plotting_config["mask_cmap"] = "blue"
mask = np.zeros((1024, 1024))
mask[500, :] = 1
fig, _ = Images(
data=RNG.random((1024, 1024)),
output_dir=tmp_path,
filename="mask_dilation",
masked_array=mask,
**plotting_config,
).plot_and_save()
return fig


@pytest.mark.parametrize(
("n", "expected"),
[
Expand Down
2 changes: 1 addition & 1 deletion topostats/default_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ plotting:
savefig_dpi: 600 # Options : null (defaults to the value in topostats/plotting_dictionary.yaml), see https://afm-spm.github.io/TopoStats/main/configuration.html#further-customisation and https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.savefig.html
pixel_interpolation: null # Options : https://matplotlib.org/stable/gallery/images_contours_and_fields/interpolation_methods.html
image_set: core # Options : all, core
zrange: [-2, 6] # low and high height range for core images (can take [null, null]). low <= high
zrange: [null, null] # low and high height range for core images (can take [null, null]). low <= high
colorbar: true # Options : true, false
axes: true # Options : true, false (due to off being a bool when parsed)
num_ticks: [null, null] # Number of ticks to have along the x and y axes. Options : null (auto) or integer > 1
Expand Down
8 changes: 0 additions & 8 deletions topostats/plottingfuncs.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,6 @@ def save_figure(self):
vmax=self.zrange[1],
)
if isinstance(self.masked_array, np.ndarray):
# self.masked_array[self.masked_array != 0] = 1
# If the image is too large for singles to be resolved in the mask, then dilate the mask proportionally
# to image size to enable clear viewing.
# if np.max(self.masked_array.shape) > 500:
# dilation_strength = int(np.max(self.masked_array.shape) / 256)
# self.masked_array = dilate_binary_image(
# binary_image=self.masked_array, dilation_iterations=dilation_strength
# )
mask = np.ma.masked_where(self.masked_array == 0, self.masked_array)
ax.imshow(
mask,
Expand Down

0 comments on commit af7a2d9

Please sign in to comment.