ronantakizawa/codereview-bench huggingface dataset:
Repository: Project-MONAI/MONAI
File: tests/test_barlow_twins_loss.py (Python)

Code before:
    def test_result(self, input_param, input_data, expected_val):
        barlowtwinsloss = BarlowTwinsLoss(**input_param)
        result = barlowtwinsloss(**input_data)
        np.testing.assert_allclose(result.detach().cpu().numpy(), expected_val, atol=1e-4, rtol=1e-4)

    def test_ill_shape(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaisesRegex(ValueError, ""):
            loss(torch.ones((1, 2, 3)), torch.ones((1, 1, 2, 3)))

    def test_ill_batch_size(self):
        loss = BarlowTwinsLoss(lambd=5e-3, batch_size=1)
        with self.assertRaisesRegex(ValueError, ""):
            loss(torch.ones((1, 2)), torch.ones((1, 2)))

    def test_with_cuda(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        i = torch.ones((2, 10))
        j = torch.ones((2, 10))
        if torch.cuda.is_available():
            i = i.cuda()
            j = j.cuda()
        output = loss(i, j)
        np.testing.assert_allclose(output.detach().cpu().numpy(), 10.0, atol=1e-4, rtol=1e-4)

    def check_warning_rasied(self):
        with self.assertWarns(Warning):
            BarlowTwinsLoss(lambd=5e-3, batch_size=1)


if __name__ == "__main__":
    unittest.main()


Review comment (suggestion):
```suggestion
    def check_warning_raised(self):
```

Code after:
    def test_result(self, input_param, input_data, expected_val):
        barlowtwinsloss = BarlowTwinsLoss(**input_param)
        result = barlowtwinsloss(**input_data)
        np.testing.assert_allclose(result.detach().cpu().numpy(), expected_val, atol=1e-4, rtol=1e-4)

    def test_ill_shape(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaises(ValueError):
            loss(torch.ones((1, 2, 3)), torch.ones((1, 1, 2, 3)))

    def test_ill_batch_size(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaises(ValueError):
            loss(torch.ones((1, 2)), torch.ones((1, 2)))

    def test_with_cuda(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        i = torch.ones((2, 10))
        j = torch.ones((2, 10))
        if torch.cuda.is_available():
            i = i.cuda()
            j = j.cuda()
        output = loss(i, j)
        np.testing.assert_allclose(output.detach().cpu().numpy(), 10.0, atol=1e-4, rtol=1e-4)

    def check_warning_raised(self):
        with self.assertWarns(Warning):
            BarlowTwinsLoss(lambd=5e-3, batch_size=1)


if __name__ == "__main__":
    unittest.main()
Repository: Project-MONAI/MONAI
File: tests/test_barlow_twins_loss.py (Python)

Code before:
                    [-2 / 3, -1.0, 7 / 3, 1 / 3],
                    [1 / 3, 0.0, 1 / 3, -2 / 3],
                ]
            ),
        },
        1.4736,
    ],
]


class TestBarlowTwinsLoss(unittest.TestCase):

    @parameterized.expand(TEST_CASES)
    def test_result(self, input_param, input_data, expected_val):
        barlowtwinsloss = BarlowTwinsLoss(**input_param)
        result = barlowtwinsloss(**input_data)
        np.testing.assert_allclose(result.detach().cpu().numpy(), expected_val, atol=1e-4, rtol=1e-4)

    def test_ill_shape(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaises(ValueError):
            loss(torch.ones((1, 2, 3)), torch.ones((1, 1, 2, 3)))

    def test_ill_batch_size(self):
        loss = BarlowTwinsLoss(lambd=5e-3, batch_size=1)
        with self.assertRaisesRegex(ValueError, ""):
            loss(torch.ones((1, 2)), torch.ones((1, 2)))

    def test_with_cuda(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        i = torch.ones((2, 10))
        j = torch.ones((2, 10))
        if torch.cuda.is_available():
            i = i.cuda()
            j = j.cuda()
        output = loss(i, j)
        np.testing.assert_allclose(output.detach().cpu().numpy(), 10.0, atol=1e-4, rtol=1e-4)

    def check_warning_raised(self):
        with self.assertWarns(Warning):
            BarlowTwinsLoss(lambd=5e-3, batch_size=1)


if __name__ == "__main__":
    unittest.main()


Review comment (suggestion):
```suggestion
        with self.assertRaises(ValueError):
```
The same here.

Code after:
                    [-2 / 3, -1.0, 7 / 3, 1 / 3],
                    [1 / 3, 0.0, 1 / 3, -2 / 3],
                ]
            ),
        },
        1.4736,
    ],
]


class TestBarlowTwinsLoss(unittest.TestCase):

    @parameterized.expand(TEST_CASES)
    def test_result(self, input_param, input_data, expected_val):
        barlowtwinsloss = BarlowTwinsLoss(**input_param)
        result = barlowtwinsloss(**input_data)
        np.testing.assert_allclose(result.detach().cpu().numpy(), expected_val, atol=1e-4, rtol=1e-4)

    def test_ill_shape(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaises(ValueError):
            loss(torch.ones((1, 2, 3)), torch.ones((1, 1, 2, 3)))

    def test_ill_batch_size(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        with self.assertRaises(ValueError):
            loss(torch.ones((1, 2)), torch.ones((1, 2)))

    def test_with_cuda(self):
        loss = BarlowTwinsLoss(lambd=5e-3)
        i = torch.ones((2, 10))
        j = torch.ones((2, 10))
        if torch.cuda.is_available():
            i = i.cuda()
            j = j.cuda()
        output = loss(i, j)
        np.testing.assert_allclose(output.detach().cpu().numpy(), 10.0, atol=1e-4, rtol=1e-4)

    def check_warning_raised(self):
        with self.assertWarns(Warning):
            BarlowTwinsLoss(lambd=5e-3, batch_size=1)


if __name__ == "__main__":
    unittest.main()
Repository: Project-MONAI/MONAI
File: tests/test_get_package_version.py (Python)

Code before:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest

from monai.utils.module import get_package_version


class TestGetVersion(unittest.TestCase):

    def test_default(self):
        output = get_package_version("42foobarnoexist")
        self.assertIn("UNKNOWN", output)

        output = get_package_version("numpy")
        self.assertIn("UNKNOWN", output)

    def test_msg(self):
        output = get_package_version("42foobarnoexist", "test")
        self.assertIn("test", output)


if __name__ == "__main__":
    unittest.main()


Review comment (suggestion):
```suggestion
        self.assertNotIn("UNKNOWN", output)
```

Code after:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest

from monai.utils.module import get_package_version


class TestGetVersion(unittest.TestCase):

    def test_default(self):
        output = get_package_version("42foobarnoexist")
        self.assertIn("UNKNOWN", output)

        output = get_package_version("numpy")
        self.assertNotIn("UNKNOWN", output)

    def test_msg(self):
        output = get_package_version("42foobarnoexist", "test")
        self.assertIn("test", output)


if __name__ == "__main__":
    unittest.main()
Repository: Project-MONAI/MONAI
File: tests/test_map_and_generate_sampling_centers.py (Python)

Code before:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest

import numpy as np
from parameterized import parameterized

from monai.transforms import map_classes_to_indices, map_and_generate_sampling_centers

class TestMapAndGenerateSamplingCenters(unittest.TestCase):

    @parameterized.expand([TEST_CASE_1, TEST_CASE_2])
    def test_map_and_generate_sampling_centers(self, input_data):
        label = np.array([[[0, 1, 2], [2, 0, 1], [1, 2, 0]]])
        spatial_size = (2, 2)
        num_samples = 3
        label_spatial_shape = label.shape
        indices = map_classes_to_indices(**input_data)
        centers = map_and_generate_sampling_centers(label, spatial_size, num_samples, label_spatial_shape, indices)
        self.assertEqual(len(centers), num_samples)

if __name__ == '__main__':
    unittest.main()


Review comment (suggestion):
```suggestion
from monai.transforms import map_and_generate_sampling_centers
```

Code after:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest
from copy import deepcopy

import numpy as np
from parameterized import parameterized

from monai.transforms import map_and_generate_sampling_centers
from monai.utils.misc import set_determinism
from tests.utils import TEST_NDARRAYS, assert_allclose

TEST_CASE_1 = [
    # test Argmax data
    {
        "label": (np.array([[[0, 1, 2], [2, 0, 1], [1, 2, 0]]])),
        "spatial_size": [2, 2, 2],
        "num_samples": 2,
        "label_spatial_shape": [3, 3, 3],
        "num_classes": 3,
        "image": None,
        "ratios": [0, 1, 2],
        "image_threshold": 0.0,
    },
    tuple,
    2,
    3,
]

TEST_CASE_2 = [
    {
        "label": (
            np.array(
Repository: Project-MONAI/MONAI
File: tests/test_map_and_generate_sampling_centers.py (Python)

Code before:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest

import numpy as np
from parameterized import parameterized

from monai.transforms import map_classes_to_indices, map_and_generate_sampling_centers

class TestMapAndGenerateSamplingCenters(unittest.TestCase):

    @parameterized.expand([TEST_CASE_1, TEST_CASE_2])
    def test_map_and_generate_sampling_centers(self, input_data):
        label = np.array([[[0, 1, 2], [2, 0, 1], [1, 2, 0]]])
        spatial_size = (2, 2)
        num_samples = 3
        label_spatial_shape = label.shape
        indices = map_classes_to_indices(**input_data)
        centers = map_and_generate_sampling_centers(label, spatial_size, num_samples, label_spatial_shape, indices)
        self.assertEqual(len(centers), num_samples)

if __name__ == '__main__':
    unittest.main()


Review comment (suggestion):
```suggestion
    def test_map_and_generate_sampling_centers(self, input_data, expected_type, expected_count, expected_shape):
```

Code after:
# Copyright (c) MONAI Consortium
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#     http://www.apache.org/licenses/LICENSE-2.0
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

import unittest
from copy import deepcopy

import numpy as np
from parameterized import parameterized

from monai.transforms import map_and_generate_sampling_centers
from monai.utils.misc import set_determinism
from tests.utils import TEST_NDARRAYS, assert_allclose

TEST_CASE_1 = [
    # test Argmax data
    {
        "label": (np.array([[[0, 1, 2], [2, 0, 1], [1, 2, 0]]])),
        "spatial_size": [2, 2, 2],
        "num_samples": 2,
        "label_spatial_shape": [3, 3, 3],
        "num_classes": 3,
        "image": None,
        "ratios": [0, 1, 2],
        "image_threshold": 0.0,
    },
    tuple,
    2,
    3,
]

TEST_CASE_2 = [
    {
        "label": (
            np.array(
                [
                    [[1, 0, 0], [0, 1, 0], [0, 0, 1]],
                    [[0, 1, 0], [0, 0, 1], [1, 0, 0]],
                    [[0, 0, 1], [1, 0, 0], [0, 1, 0]],
                ]
Repository: Project-MONAI/MONAI
File: monai/transforms/io/array.py (Python)

Code before:
        """
        Args:
            input_path (str): The path of the input image file.
            output_path (str): The path of the output image file.
        """
        log_data = {"input": input_path, "output": output_path}
        try:
            with self.mapping_json_path.open("r") as f:
                existing_log_data = json.load(f)
        except FileNotFoundError:
            existing_log_data = []

        existing_log_data.append(log_data)

        with self.mapping_json_path.open("w") as f:
            json.dump(existing_log_data, f, indent=4)

    def __call__(self, img: MetaTensor):
        """
        Args:
            img (MetaTensor): The input image with metadata.
        """
        if "saved_to" not in img.meta:
            raise KeyError("Missing 'saved_to' key in metadata. Check SaveImage savepath_in_metadict.")

        input_path = img.meta["filename_or_obj"]
        output_path = img.meta["saved_to"]
        self.write_json(input_path, output_path)
        return img


Review comment (suggestion):
For the string `filename_or_obj`, I would recommand to use the enum [ImageMetaKey.FILENAME_OR_OBJ](https://github.com/Project-MONAI/MONAI/blob/2e53df78e580131046dc8db7f7638063db1f5045/monai/utils/misc.py#L577). For the `saved_to` string, @KumoLiu  I would suggest to create a new enum to record metadata keys that are optional. What do you think about this?

Thanks  
Bin



Code after:

    Args:
        mapping_file_path (Path or str): Path to the JSON file where the mappings will be saved.
    """

    def __init__(self, mapping_file_path: Path | str = "mapping.json"):
        self.mapping_file_path = Path(mapping_file_path)

    def __call__(self, img: NdarrayOrTensor):
        """
        Args:
            img: The input image with metadata.
        """
        if isinstance(img, MetaTensor):
            meta_data = img.meta

        if MetaKeys.SAVED_TO not in meta_data:
            raise KeyError(
                "Missing 'saved_to' key in metadata. Check SaveImage argument 'savepath_in_metadict' is True."
            )

        input_path = meta_data[Key.FILENAME_OR_OBJ]
        output_path = meta_data[MetaKeys.SAVED_TO]
        log_data = {"input": input_path, "output": output_path}

        if has_filelock:
            with FileLock(str(self.mapping_file_path) + ".lock"):
                self._write_to_file(log_data)
        else:
            self._write_to_file(log_data)
        return img

    def _write_to_file(self, log_data):
        try:
            with self.mapping_file_path.open("r") as f:
                existing_log_data = json.load(f)
        except (FileNotFoundError, json.JSONDecodeError):
            existing_log_data = []
        existing_log_data.append(log_data)
        with self.mapping_file_path.open("w") as f:
            json.dump(existing_log_data, f, indent=4)
Repository: Project-MONAI/MONAI
File: monai/transforms/io/array.py (Python)

Code before:
        """
        Args:
            input_path (str): The path of the input image file.
            output_path (str): The path of the output image file.
        """
        log_data = {"input": input_path, "output": output_path}

        with self.lock:
            try:
                with self.mapping_json_path.open("r") as f:
                    existing_log_data = json.load(f)
            except (FileNotFoundError, json.JSONDecodeError):
                existing_log_data = []

            existing_log_data.append(log_data)

            with self.mapping_json_path.open("w") as f:
                json.dump(existing_log_data, f, indent=4)

    def __call__(self, img: MetaTensor):
        """
        Args:
            img (MetaTensor): The input image with metadata.
        """
        if "saved_to" not in img.meta:
            raise KeyError("Missing 'saved_to' key in metadata. Check SaveImage savepath_in_metadict.")
        input_path = img.meta["filename_or_obj"]
        output_path = img.meta["saved_to"]
        self.write_json(input_path, output_path)
        return img


Review comment (suggestion):
```suggestion
            raise KeyError("Missing 'saved_to' key in metadata. Check SaveImage argument 'savepath_in_metadict' is True.")
```

Code after:
    """

    def __init__(self, mapping_file_path: Path | str = "mapping.json"):
        self.mapping_file_path = Path(mapping_file_path)

    def __call__(self, img: NdarrayOrTensor):
        """
        Args:
            img: The input image with metadata.
        """
        if isinstance(img, MetaTensor):
            meta_data = img.meta

        if MetaKeys.SAVED_TO not in meta_data:
            raise KeyError(
                "Missing 'saved_to' key in metadata. Check SaveImage argument 'savepath_in_metadict' is True."
            )

        input_path = meta_data[Key.FILENAME_OR_OBJ]
        output_path = meta_data[MetaKeys.SAVED_TO]
        log_data = {"input": input_path, "output": output_path}

        if has_filelock:
            with FileLock(str(self.mapping_file_path) + ".lock"):
                self._write_to_file(log_data)
        else:
            self._write_to_file(log_data)
        return img

    def _write_to_file(self, log_data):
        try:
            with self.mapping_file_path.open("r") as f:
                existing_log_data = json.load(f)
        except (FileNotFoundError, json.JSONDecodeError):
            existing_log_data = []
        existing_log_data.append(log_data)
        with self.mapping_file_path.open("w") as f:
            json.dump(existing_log_data, f, indent=4)
Repository: Project-MONAI/MONAI
File: monai/apps/generation/maisi/utils/morphological_ops.py (Python)

Code before:
        filter_size: erosion filter size, has to be odd numbers, default to be 3.
        pad_value: the filled value for padding. We need to pad the input before filtering to keep the output with the same size as input. Usually use default value and not changed.

    Return:
        eroded mask, [N,C,M,N] or [N,C,M,N,P] torch tensor
    """

    output = get_morphological_filter_result_t(mask_t, filter_size, pad_value)

    # Set output values based on the minimum value within the structuring element
    output = torch.where(output == 1.0, 1.0, 0.0)

    return output


def dilate_t(mask_t: Tensor, filter_size: int | Sequence[int] = 3, pad_value: float = 0.0) -> Tensor:
    """
    Dilate 2D/3D binary mask with data type as torch tensor.

    Args:
        mask_t: input 2D/3D binary mask, [N,C,M,N] or [N,C,M,N,P] torch tensor.
        filter_size: dilation filter size, has to be odd numbers, default to be 3.
        pad_value: the filled value for padding. We need to pad the input before filtering to keep the output with the same size as input. Usually use default value and not changed.

    Return:
        dilated mask, [N,C,M,N] or [N,C,M,N,P] torch tensor
    """
    output = get_morphological_filter_result_t(mask_t, filter_size, pad_value)

    # Set output values based on the minimum value within the structuring element
    output = torch.where(output > 0, 1.0, 0.0)

    return output


Review comment (suggestion):
```suggestion
        Tensor: dilated mask, same shape as input.
```

Code after:
        filter_size: erosion filter size, has to be odd numbers, default to be 3.
        pad_value: the filled value for padding. We need to pad the input before filtering
                   to keep the output with the same size as input. Usually use default value
                   and not changed.

    Return:
        Tensor: eroded mask, same shape as input.
    """

    output = get_morphological_filter_result_t(mask_t, filter_size, pad_value)

    # Set output values based on the minimum value within the structuring element
    output = torch.where(torch.abs(output - 1.0) < 1e-7, 1.0, 0.0)

    return output


def dilate_t(mask_t: Tensor, filter_size: int | Sequence[int] = 3, pad_value: float = 0.0) -> Tensor:
    """
    Dilate 2D/3D binary mask with data type as torch tensor.

    Args:
        mask_t: input 2D/3D binary mask, [N,C,M,N] or [N,C,M,N,P] torch tensor.
        filter_size: dilation filter size, has to be odd numbers, default to be 3.
        pad_value: the filled value for padding. We need to pad the input before filtering
                   to keep the output with the same size as input. Usually use default value
                   and not changed.

    Return:
        Tensor: dilated mask, same shape as input.
    """
    output = get_morphological_filter_result_t(mask_t, filter_size, pad_value)

    # Set output values based on the minimum value within the structuring element
    output = torch.where(output > 0, 1.0, 0.0)

    return output