Skip to content

Commit

Permalink
fix: make Image.clone internal (#725)
Browse files Browse the repository at this point in the history
Closes #626

### Summary of Changes

`Image.clone` is now internal. It should never be called explicitly by
users, since our implementation already takes care of ensuring
immutability.
  • Loading branch information
lars-reimann authored May 5, 2024
1 parent 2960d35 commit 215a472
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/safeds/data/image/containers/_empty_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def __new__(cls) -> Self:
def _create_image_list(images: list[Tensor], indices: list[int]) -> ImageList:
raise NotImplementedError

def clone(self) -> ImageList:
def _clone(self) -> ImageList:
return _EmptyImageList()

def __eq__(self, other: object) -> bool:
Expand Down
2 changes: 1 addition & 1 deletion src/safeds/data/image/containers/_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def from_files(path: str | Path | Sequence[str | Path]) -> ImageList:
return _MultiSizeImageList._create_image_list(image_tensors, indices)

@abstractmethod
def clone(self) -> ImageList:
def _clone(self) -> ImageList:
"""
Clone your ImageList to a new instance.
Expand Down
10 changes: 5 additions & 5 deletions src/safeds/data/image/containers/_multi_size_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ def _create_image_list(images: list[Tensor], indices: list[int]) -> ImageList:

return image_list

def clone(self) -> ImageList:
def _clone(self) -> ImageList:
cloned_image_list = self._clone_without_image_dict()
for image_list_size, image_list in self._image_list_dict.items():
cloned_image_list._image_list_dict[image_list_size] = image_list.clone()
cloned_image_list._image_list_dict[image_list_size] = image_list._clone()
return cloned_image_list

def _clone_without_image_dict(self) -> _MultiSizeImageList:
Expand Down Expand Up @@ -246,7 +246,7 @@ def _add_image_tensor(self, image_tensor: Tensor, index: int) -> ImageList:
if index in self._indices_to_image_size_dict:
raise DuplicateIndexError(index)

image_list = self.clone()._as_multi_size_image_list()
image_list = self._clone()._as_multi_size_image_list()
size = (image_tensor.size(dim=2), image_tensor.size(dim=1))
image_list._indices_to_image_size_dict[index] = size

Expand Down Expand Up @@ -296,7 +296,7 @@ def add_images(self, images: list[Image] | ImageList) -> ImageList:
images_with_size[size] = [image]
indices_for_images_with_size[size] = [current_index]
current_index += 1
image_list = self.clone()._as_multi_size_image_list()
image_list = self._clone()._as_multi_size_image_list()
smallest_channel = max_channel = self.channel
for size, ims in (images_with_size | image_list_with_size).items():
new_indices = indices_for_images_with_size[size]
Expand Down Expand Up @@ -434,7 +434,7 @@ def shuffle_images(self) -> ImageList:
random.shuffle(new_indices)
current_index = 0
for image_list_key, image_list_original in self._image_list_dict.items():
new_image_list = image_list_original.clone()._as_single_size_image_list()
new_image_list = image_list_original._clone()._as_single_size_image_list()
new_image_list._tensor_positions_to_indices = new_indices[
current_index : current_index + len(image_list_original)
]
Expand Down
4 changes: 2 additions & 2 deletions src/safeds/data/image/containers/_single_size_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def _create_image_list(images: list[Tensor], indices: list[int]) -> ImageList:

return image_list

def clone(self) -> ImageList:
def _clone(self) -> ImageList:
cloned_image_list = self._clone_without_tensor()
cloned_image_list._tensor = self._tensor.detach().clone()
return cloned_image_list
Expand Down Expand Up @@ -565,7 +565,7 @@ def remove_duplicate_images(self) -> ImageList:
return image_list

def shuffle_images(self) -> ImageList:
image_list = self.clone()._as_single_size_image_list()
image_list = self._clone()._as_single_size_image_list()
random.shuffle(image_list._tensor_positions_to_indices)
image_list._indices_to_tensor_positions = image_list._calc_new_indices_to_tensor_positions()
return image_list
Expand Down
36 changes: 18 additions & 18 deletions tests/safeds/data/image/containers/test_image_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def test_from_files(self, resource_path1: str, resource_path2: str, resource_pat
image3_with_expected_channel = image3.change_channel(expected_channel)

# Test clone
image_list_clone = image_list.clone()
image_list_clone = image_list._clone()
assert image_list_clone is not image_list
assert image_list_clone == image_list

Expand Down Expand Up @@ -732,7 +732,7 @@ class TestShuffleImages:
def test_shuffle_images(self, resource_path: list[str], snapshot_png_image_list: SnapshotAssertion) -> None:
torch.set_default_device(_get_device())
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
random.seed(420)
image_list_shuffled = image_list_original.shuffle_images()
random.seed()
Expand Down Expand Up @@ -816,7 +816,7 @@ def test_all_transform_methods(
resolve_resource_path(resource_path3),
],
)
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()

if isinstance(attributes, list):
image_list_transformed = getattr(image_list_original, method)(*attributes)
Expand Down Expand Up @@ -861,7 +861,7 @@ def test_should_add_noise(
torch.set_default_device(torch.device("cpu"))
torch.manual_seed(0)
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
image_list_noise = image_list_original.add_noise(standard_deviation)
assert image_list_noise == snapshot_png_image_list
assert image_list_original is not image_list_clone
Expand Down Expand Up @@ -918,7 +918,7 @@ def test_should_not_adjust_color_balance_channel_1(
resource_path: list[str],
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path)).change_channel(1)
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Color adjustment will not have an affect on grayscale images with only one channel.",
Expand Down Expand Up @@ -1032,7 +1032,7 @@ def test_should_raise_standard_deviation(
standard_deviation: float,
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(
OutOfBoundsError,
match=rf"standard_deviation \(={standard_deviation}\) is not inside \[0, \u221e\)\.",
Expand All @@ -1053,7 +1053,7 @@ def test_should_raise(
factor: float,
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(OutOfBoundsError, match=r"factor \(=-1\) is not inside \[0, \u221e\)."):
image_list_original.adjust_brightness(factor)
assert image_list_original == image_list_clone
Expand All @@ -1063,7 +1063,7 @@ def test_should_not_brighten(
resource_path: list[str],
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Brightness adjustment factor is 1.0, this will not make changes to the images.",
Expand All @@ -1085,7 +1085,7 @@ def test_should_raise(
factor: float,
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(OutOfBoundsError, match=r"factor \(=-1\) is not inside \[0, \u221e\)."):
image_list_original.adjust_contrast(factor)
assert image_list_original == image_list_clone
Expand All @@ -1095,7 +1095,7 @@ def test_should_not_adjust(
resource_path: list[str],
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Contrast adjustment factor is 1.0, this will not make changes to the images.",
Expand All @@ -1117,7 +1117,7 @@ def test_should_raise(
factor: float,
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(OutOfBoundsError, match=r"factor \(=-1\) is not inside \[0, \u221e\)."):
image_list_original.adjust_color_balance(factor)
assert image_list_original == image_list_clone
Expand All @@ -1127,7 +1127,7 @@ def test_should_not_adjust_color_balance_factor_1(
resource_path: list[str],
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Color adjustment factor is 1.0, this will not make changes to the images.",
Expand All @@ -1140,7 +1140,7 @@ class TestBlur:

def test_should_raise_radius_out_of_bounds(self, resource_path: str) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(
OutOfBoundsError,
match=rf"radius \(=-1\) is not inside \[0, {'0' if isinstance(image_list_original, _EmptyImageList) else min(*image_list_original.widths, *image_list_original.heights) - 1}\].",
Expand All @@ -1161,7 +1161,7 @@ def test_should_raise_radius_out_of_bounds(self, resource_path: str) -> None:

def test_should_not_blur(self, resource_path: str) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Blur radius is 0, this will not make changes to the images.",
Expand All @@ -1183,7 +1183,7 @@ def test_should_raise(
factor: float,
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.raises(OutOfBoundsError, match=r"factor \(=-1\) is not inside \[0, \u221e\)."):
image_list_original.sharpen(factor)
assert image_list_original == image_list_clone
Expand All @@ -1193,7 +1193,7 @@ def test_should_not_adjust(
resource_path: list[str],
) -> None:
image_list_original = ImageList.from_files(resolve_resource_path(resource_path))
image_list_clone = image_list_original.clone()
image_list_clone = image_list_original._clone()
with pytest.warns(
UserWarning,
match="Sharpen factor is 1.0, this will not make changes to the images.",
Expand Down Expand Up @@ -1234,8 +1234,8 @@ def test_from_files(self) -> None:
assert ImageList.from_files([tmpdir]) == _EmptyImageList()

def test_clone(self) -> None:
assert _EmptyImageList() == _EmptyImageList().clone()
assert _EmptyImageList() is _EmptyImageList().clone() # Singleton
assert _EmptyImageList() == _EmptyImageList()._clone()
assert _EmptyImageList() is _EmptyImageList()._clone() # Singleton

def test_repr_png(self) -> None:
with pytest.raises(TypeError, match=r"You cannot display an empty ImageList"):
Expand Down

0 comments on commit 215a472

Please sign in to comment.