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

Tests for the image_io sub-module #48

Merged
merged 10 commits into from
Mar 14, 2024
10 changes: 5 additions & 5 deletions brainglobe_utils/image_io/load.py
Original file line number Diff line number Diff line change
@@ -44,8 +44,8 @@ def load_any(
Parameters
----------
src_path : str
Can be the path of a nifty file, tiff file, tiff files folder, or text
file containing a list of paths.
Can be the path of a nifty file, nrrd file, tiff file, tiff files
folder, or text file containing a list of paths.

x_scaling_factor : float, optional
The scaling of the brain along the x dimension (applied on loading
@@ -401,8 +401,7 @@ def load_image_series(
n_free_cpus=2,
):
"""
Load a brain from a sequence of files specified in a text file containing
an ordered list of paths.
Load a brain from a sequence of image paths.

Parameters
----------
@@ -601,7 +600,8 @@ def get_size_image_from_file_paths(file_path, file_extension="tif"):
Parameters
----------
file_path : str
File containing file_paths in a text file, or as a list.
Filepath of text file containing paths of all 2D files, or
filepath of a directory containing all 2D files.

file_extension : str, optional
Optional file extension (if a directory is passed).
8 changes: 4 additions & 4 deletions brainglobe_utils/image_io/save.py
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ def to_nii(img, dest_path, scale=None, affine_transform=None):
A nifty image object or numpy array representing a brain.

dest_path : str
The path where to save the brain.
The file path where to save the brain.

scale : tuple of floats, optional
A tuple of floats to indicate the 'zooms' of the nifty image.
@@ -50,7 +50,7 @@ def to_tiff(img_volume, dest_path, photometric="minisblack"):
The image to be saved.

dest_path : str
Where to save the tiff stack.
The file path where to save the tiff stack.

photometric: str
Color space of image (to pass to tifffile.imwrite)
@@ -64,7 +64,7 @@ def to_tiffs(img_volume, path_prefix, path_suffix="", extension=".tif"):
"""
Save the image volume (numpy array) as a sequence of tiff planes.
Each plane will have a filepath of the following format:
pathprefix_zeroPaddedIndex_suffix.tif
{path_prefix}_{zeroPaddedIndex}{path_suffix}{extension}

Parameters
----------
@@ -100,7 +100,7 @@ def to_nrrd(img_volume, dest_path):
The image to be saved.

dest_path : str
Where to save the nrrd image.
The file path where to save the nrrd image.
"""
dest_path = str(dest_path)
nrrd.write(dest_path, img_volume)
304 changes: 262 additions & 42 deletions tests/tests/test_image_io.py
Original file line number Diff line number Diff line change
@@ -1,63 +1,283 @@
import os
import random

import numpy as np
import pytest
from tifffile import tifffile

from brainglobe_utils.image_io import load, save, utils


@pytest.fixture()
def layer():
def array_2d():
"""Create a 4x4 array of 32-bit integers"""
return np.tile(np.array([1, 2, 3, 4], dtype=np.int32), (4, 1))


@pytest.fixture()
def start_array(layer):
volume = np.dstack((layer, 2 * layer, 3 * layer, 4 * layer))
def array_3d(array_2d):
"""Create a 4x4x4 array of 32-bit integers"""
volume = np.stack((array_2d, 2 * array_2d, 3 * array_2d, 4 * array_2d))
return volume


def test_tiff_io(tmpdir, layer):
folder = str(tmpdir)
dest_path = os.path.join(folder, "layer.tiff")
tifffile.imwrite(dest_path, layer)
reloaded = tifffile.imread(dest_path)
assert (reloaded == layer).all()


def test_to_tiffs(tmpdir, start_array):
folder = str(tmpdir)
save.to_tiffs(start_array, os.path.join(folder, "start_array"))
reloaded_array = load.load_from_folder(folder, 1, 1, 1)
assert (reloaded_array == start_array).all()


def test_load_img_sequence(tmpdir, start_array):
folder = str(tmpdir.mkdir("sub"))
save.to_tiffs(start_array, os.path.join(folder, "start_array"))
img_sequence_file = tmpdir.join("imgs_file.txt")
img_sequence_file.write(
"\n".join(
[
os.path.join(folder, fname)
for fname in sorted(os.listdir(folder))
]
)
@pytest.fixture(params=["2D", "3D"])
def image_array(request, array_2d, array_3d):
"""Create both a 2D and 3D array of 32-bit integers"""
if request.param == "2D":
return array_2d
else:
return array_3d


def write_tiff_sequence_with_txt_file(txt_path, image_array, shuffle=False):
"""
Write an image array to a series of tiffs, and write a text file
containing all the tiff file paths in order (one per line).
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth to briefly mention a typical use case for that text file?
Reading this, I understood what was going on, but wasn't sure why.

[Edit:] after reviewing the rest of the PR, my understanding as to why we need this is that we support this input format, but never write to it, so we want to have this to test. Presumably this format is useful for parallel/lazy IO? Could this be a fixture just for consistency - so all our test code is fixtures or test functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's right - it's needed as we support reading from a txt file with load_img_sequence, but there's no corresponding save function. I'm not sure what the use-case is for such a text file - I guess if you have tiff files spread over multiple directories it could be useful?

I'll look at changing this into a fixture now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alessandrofelder - I'm trying to convert this to a fixture, but having problems with allowing it to be optionally shuffled like here. Is there a simple way to pass a parameter like shuffle=True to a fixture? Otherwise, I could have two fixtures - one for a shuffled text file and one for not, but they will have pretty much identical code in both unless I refactor it out into some kind of helper function. Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixtures can't take optional arguments, but can be indirectly parameterised:

import pytest

@pytest.fixture
def my_fixture(msg: str, reps: int) -> str:
    return msg * reps


@pytest.mark.parametrize(
    "msg, reps, expected",
    [
        pytest.param("Hi", 2, "HiHi"),
        pytest.param("Hi", 1, "Hi"),
    ],
)
def test_thing(my_fixture, expected):
    assert my_fixture == expected

will pass, for example. However if you change reps: int to reps: int = 2 in the fixture definition, the invoking pytest will fail.

However I don't think write_tiff_sequence_with_txt_file can be made into a fixture (depending on how you're using it). I'm prepared to be proven wrong though 😅 But in my understanding, fixtures have to either:

  • Return an object that can be constructed a-priori to the test running.
  • Be a sequence of executable steps that serves as either the setup or teardown of a test.

Here, write_tiff_sequence_with_txt_file needs to be run in the middle of a test, so it has to be a function, and can't be a fixture. You could try redesigning it as a setup fixture though - assuming you're only ever using it at the start of a test, before anything else runs:

@pytest.fixture
def write_tiff_sequence_with_txt_file(txt_file, input_array, shuffle):
    # Do things prior to running a test
    yield # This is where the test will run


@pytest.mark.usefixtures("write_tiff_sequency_with_txt_file")
@pytest.mark.parametrize(
    "txt_file, input_array, shuffle, other_args_used_by_test",
    [
        ["foo.txt", [1,2,3], True, "some_fancy_args"],
    ],
)
def test_things(txt_file, input_array, shuffle, other_args_used_by_test) -> None:
    # write_tiff_sequence_with_txt_file will run here before the actual test instructions
    assert other_args_used_by_test
    # Note that txt_file, input_array, shuffle don't actually have to be in the test-function declaration,
    # test_things(other_args_used_by_test) also works fine.
    # But if you DO include them, the values of the variables will be usable within the test-function scope too.
    # IE the line
    assert (
        txt_file is not None
    )  
    # Only works if test_things(txt_file, other_args_used_by_test) is used as the definition, 
    # but not if test_things(other_args_used_by_test) is used instead.

Otherwise, I don't think it can be "fixture-ised".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info @willGraham01! That makes it a lot clearer.
I think I could redesign it as a setup fixture, but this would mean I'd need a separate test case for txt files with load_any, rather than having it as a parameter here

As we support loading txt files in image_io via load_any and load_img_sequence, perhaps it would make sense for this to be moved into image_io to allow saving this kind of file also? Then the test functions could simply call this?

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. Yea fixture parameters are tricky!

In this specific case, IIUC we need the shuffle=True part for testing the sort argument of load_img_sequence only, whereas we will need the shuffle=False case in the source code (as part of addressing #50). I don't think we'd ever want to intentionally shuffle the order of paths in a text file in our source code?

So I'd suggest

  • keeping write_tiff_sequence_with_txt_file not a fixture
  • splitting the shuffle part into a fixture that overwrites the textfile with a shuffled version of it.
  • as part of Add a save_any function to image_io #50, move write_tiff_sequence_with_txt_file to the source code

Is that sensible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me! I'll add these points to the issue + split the shuffle part now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alessandrofelder I split shuffle into a fixture now. Could you take a look + merge if you're happy with it? Then I'll work on #50


The tiff sequence will be saved to a sub-folder inside the same folder
as the text file.

Parameters
----------
txt_path : pathlib.Path
Filepath of text file to create
image_array : np.ndarray
Image to write as sequence of tiffs
shuffle : bool
Whether to shuffle the order of filepaths in the text file
"""
directory = txt_path.parent

# Write tiff sequence to sub-folder
sub_dir = directory / "sub"
sub_dir.mkdir()
save.to_tiffs(image_array, str(sub_dir / "image_array"))

# Write txt file containing all tiff file paths (one per line)
tiff_paths = sorted(sub_dir.iterdir())
if shuffle:
random.Random(4).shuffle(tiff_paths)
txt_path.write_text(
"\n".join([str(sub_dir / fname) for fname in tiff_paths])
)
reloaded_array = load.load_img_sequence(str(img_sequence_file), 1, 1, 1)
assert (reloaded_array == start_array).all()


def test_to_nii(tmpdir, start_array): # Also tests load_nii
folder = str(tmpdir)
nii_path = os.path.join(folder, "test_array.nii")
save.to_nii(start_array, nii_path)
assert (load.load_nii(nii_path).get_fdata() == start_array).all()
def save_any(file_path, image_array):
"""
K-Meech marked this conversation as resolved.
Show resolved Hide resolved
Save image_array to given file path, using the save function matching
its file extension

Parameters
----------
file_path : pathlib.Path
File path of image to save
image_array : np.ndarray
Image to save
"""
if file_path.is_dir():
save.to_tiffs(image_array, str(file_path / "image_array"))

elif file_path.suffix == ".txt":
write_tiff_sequence_with_txt_file(file_path, image_array)

elif file_path.suffix == ".tif" or file_path.suffix == ".tiff":
save.to_tiff(image_array, str(file_path))

elif file_path.suffix == ".nrrd":
save.to_nrrd(image_array, str(file_path))

elif file_path.suffix == ".nii":
save.to_nii(image_array, str(file_path), scale=(1, 1, 1))


def test_tiff_io(tmp_path, image_array):
"""
Test that a 2D/3D tiff can be written and read correctly
"""
dest_path = tmp_path / "image_array.tiff"
save_any(dest_path, image_array)

def test_scale_z(start_array):
assert (
utils.scale_z(start_array, 0.5).shape[0] == start_array.shape[-1] / 2
reloaded = load.load_img_stack(str(dest_path), 1, 1, 1)
assert (reloaded == image_array).all()


@pytest.mark.parametrize(
"x_scaling_factor, y_scaling_factor, z_scaling_factor",
[(1, 1, 1), (0.5, 0.5, 1), (0.25, 0.25, 0.25)],
)
def test_3d_tiff_scaling(
tmp_path, array_3d, x_scaling_factor, y_scaling_factor, z_scaling_factor
):
"""
Test that a 3D tiff is scaled correctly on loading
"""
dest_path = tmp_path / "image_array.tiff"
save_any(dest_path, array_3d)

reloaded = load.load_img_stack(
str(dest_path), x_scaling_factor, y_scaling_factor, z_scaling_factor
)
assert reloaded.shape[0] == array_3d.shape[0] * z_scaling_factor
assert reloaded.shape[1] == array_3d.shape[1] * y_scaling_factor
assert reloaded.shape[2] == array_3d.shape[2] * x_scaling_factor


@pytest.mark.parametrize(
"load_parallel",
[
pytest.param(True, id="parallel loading"),
pytest.param(False, id="no parallel loading"),
],
)
def test_tiff_sequence_io(tmp_path, array_3d, load_parallel):
"""
Test that a 3D image can be written and read correctly as a sequence
of 2D tiffs (with or without parallel loading)
"""
save_any(tmp_path, array_3d)
reloaded_array = load.load_from_folder(
str(tmp_path), 1, 1, 1, load_parallel=load_parallel
)
assert utils.scale_z(start_array, 2).shape[0] == start_array.shape[-1] * 2
assert (reloaded_array == array_3d).all()


@pytest.mark.parametrize(
"x_scaling_factor, y_scaling_factor, z_scaling_factor",
[(1, 1, 1), (0.5, 0.5, 1), (0.25, 0.25, 0.25)],
)
def test_tiff_sequence_scaling(
tmp_path, array_3d, x_scaling_factor, y_scaling_factor, z_scaling_factor
):
"""
Test that a tiff sequence is scaled correctly on loading
"""
save_any(tmp_path, array_3d)
reloaded_array = load.load_from_folder(
str(tmp_path), x_scaling_factor, y_scaling_factor, z_scaling_factor
)

assert reloaded_array.shape[0] == array_3d.shape[0] * z_scaling_factor
assert reloaded_array.shape[1] == array_3d.shape[1] * y_scaling_factor
assert reloaded_array.shape[2] == array_3d.shape[2] * x_scaling_factor


def test_load_img_sequence_from_txt(tmp_path, array_3d):
"""
Test that a tiff sequence can be loaded from a text file containing an
ordered list of the tiff file paths (one per line)
"""
img_sequence_file = tmp_path / "imgs_file.txt"
save_any(img_sequence_file, array_3d)

# Load image from paths in text file
reloaded_array = load.load_img_sequence(str(img_sequence_file), 1, 1, 1)
assert (reloaded_array == array_3d).all()


@pytest.mark.parametrize(
"sort",
[True, False],
)
def test_sort_img_sequence_from_txt(tmp_path, array_3d, sort):
"""
Test that filepaths read from a txt file can be sorted correctly
"""
img_sequence_file = tmp_path / "imgs_file.txt"
write_tiff_sequence_with_txt_file(
img_sequence_file, array_3d, shuffle=True
)

# Load image from paths in text file
reloaded_array = load.load_img_sequence(
str(img_sequence_file), 1, 1, 1, sort=sort
)
if sort:
assert (reloaded_array == array_3d).all()
else:
assert not (reloaded_array == array_3d).all()


def test_nii_io(tmp_path, array_3d):
"""
Test that a 3D image can be written and read correctly as nii
"""
nii_path = tmp_path / "test_array.nii"
save_any(nii_path, array_3d)
assert (load.load_nii(str(nii_path)).get_fdata() == array_3d).all()


def test_nii_read_to_numpy(tmp_path, array_3d):
"""
Test that conversion of loaded nii image to an in-memory numpy array works
"""
nii_path = tmp_path / "test_array.nii"
save_any(nii_path, array_3d)

reloaded_array = load.load_nii(str(nii_path), as_array=True, as_numpy=True)
assert (reloaded_array == array_3d).all()


def test_nrrd_io(tmp_path, array_3d):
"""
Test that a 3D image can be written and read correctly as nrrd
"""
nrrd_path = tmp_path / "test_array.nrrd"
save_any(nrrd_path, array_3d)
assert (load.load_nrrd(str(nrrd_path)) == array_3d).all()


@pytest.mark.parametrize(
"file_name",
[
"test_array.tiff",
"test_array.tif",
"test_array.txt",
"test_array.nrrd",
"test_array.nii",
pytest.param("", id="dir of tiffs"),
],
)
def test_load_any(tmp_path, array_3d, file_name):
"""
Test that load_any can read all required image file types
"""
src_path = tmp_path / file_name
save_any(src_path, array_3d)

assert (load.load_any(str(src_path)) == array_3d).all()


def test_load_any_error(tmp_path):
"""
Test that load_any throws an error for an unknown file extension
"""
with pytest.raises(NotImplementedError):
load.load_any(str(tmp_path / "test.unknown"))


def test_scale_z(array_3d):
"""
Test that a 3D image can be scaled in z by float and integer values
"""
assert utils.scale_z(array_3d, 0.5).shape[0] == array_3d.shape[0] / 2
assert utils.scale_z(array_3d, 2).shape[0] == array_3d.shape[0] * 2


@pytest.mark.parametrize(
"file_name",
[
"test_array.txt",
pytest.param("", id="dir of tiffs"),
],
)
def test_image_size(tmp_path, array_3d, file_name):
"""
Test that image size can be detected from a directory of 2D tiffs, or
a text file containing the paths of a sequence of 2D tiffs
"""
file_path = tmp_path / file_name
save_any(file_path, array_3d)

image_shape = load.get_size_image_from_file_paths(str(file_path))
assert image_shape["x"] == array_3d.shape[2]
assert image_shape["y"] == array_3d.shape[1]
assert image_shape["z"] == array_3d.shape[0]