Merge branch 'dev' into vikash/NaViT

Author: ericspod

monai/apps/__init__.py

@@ -13,4 +13,13 @@
 
 from .datasets import CrossValidation, DecathlonDataset, MedNISTDataset, TciaDataset
 from .mmars import MODEL_DESC, RemoteMMARKeys, download_mmar, get_model_spec, load_from_mmar
-from .utils import SUPPORTED_HASH_TYPES, check_hash, download_and_extract, download_url, extractall, get_logger, logger
+from .utils import (
+    SUPPORTED_HASH_TYPES,
+    HashCheckError,
+    check_hash,
+    download_and_extract,
+    download_url,
+    extractall,
+    get_logger,
+    logger,
+)

monai/apps/utils.py

@@ -42,12 +42,24 @@
 else:
     tqdm, has_tqdm = optional_import("tqdm", "4.47.0", min_version, "tqdm")
 
-__all__ = ["check_hash", "download_url", "extractall", "download_and_extract", "get_logger", "SUPPORTED_HASH_TYPES"]
+__all__ = [
+    "HashCheckError",
+    "check_hash",
+    "download_url",
+    "extractall",
+    "download_and_extract",
+    "get_logger",
+    "SUPPORTED_HASH_TYPES",
+]
 
 DEFAULT_FMT = "%(asctime)s - %(levelname)s - %(message)s"
 SUPPORTED_HASH_TYPES = {"md5": hashlib.md5, "sha1": hashlib.sha1, "sha256": hashlib.sha256, "sha512": hashlib.sha512}
 
 
+class HashCheckError(ValueError):
+    pass
+
+
 def get_logger(
     module_name: str = "monai.apps",
     fmt: str = DEFAULT_FMT,
@@ -220,18 +232,15 @@ def download_url(
         HTTPError: See urllib.request.urlretrieve.
         ContentTooShortError: See urllib.request.urlretrieve.
         IOError: See urllib.request.urlretrieve.
-        RuntimeError: When the hash validation of the ``url`` downloaded file fails.
-
+        HashCheckError: When the hash validation of the ``url`` downloaded file fails.
     """
     if not filepath:
         filepath = Path(".", _basename(url)).resolve()
         logger.info(f"Default downloading to '{filepath}'")
     filepath = Path(filepath)
     if filepath.exists():
         if not check_hash(filepath, hash_val, hash_type):
-            raise RuntimeError(
-                f"{hash_type} check of existing file failed: filepath={filepath}, expected {hash_type}={hash_val}."
-            )
+            raise HashCheckError(f"{hash_type} hash check of existing file failed: {filepath=}, expected {hash_type=}.")
         logger.info(f"File exists: {filepath}, skipped downloading.")
         return
     try:
@@ -260,18 +269,20 @@ def download_url(
                 raise RuntimeError(
                     f"Download of file from {url} to {filepath} failed due to network issue or denied permission."
                 )
+            if not check_hash(tmp_name, hash_val, hash_type):
+                raise HashCheckError(
+                    f"{hash_type} hash check of downloaded file failed: {url=}, "
+                    f"{filepath=}, expected {hash_type}={hash_val}, "
+                    f"The file may be corrupted or tampered with. "
+                    "Please retry the download or verify the source."
+                )
             file_dir = filepath.parent
             if file_dir:
                 os.makedirs(file_dir, exist_ok=True)
             shutil.move(f"{tmp_name}", f"{filepath}")  # copy the downloaded to a user-specified cache.
     except (PermissionError, NotADirectoryError):  # project-monai/monai issue #3613 #3757 for windows
         pass
     logger.info(f"Downloaded: {filepath}")
-    if not check_hash(filepath, hash_val, hash_type):
-        raise RuntimeError(
-            f"{hash_type} check of downloaded file failed: URL={url}, "
-            f"filepath={filepath}, expected {hash_type}={hash_val}."
-        )
 
 
 def _extract_zip(filepath, output_dir):
@@ -325,10 +336,15 @@ def extractall(
             be False.
 
     Raises:
-        RuntimeError: When the hash validation of the ``filepath`` compressed file fails.
+        HashCheckError: When the hash validation of the ``filepath`` compressed file fails.
         NotImplementedError: When the ``filepath`` file extension is not one of [zip", "tar.gz", "tar"].
 
     """
+    filepath = Path(filepath)
+    if hash_val and not check_hash(filepath, hash_val, hash_type):
+        raise HashCheckError(
+            f"{hash_type} hash check of compressed file failed: " f"{filepath=}, expected {hash_type}={hash_val}."
+        )
     if has_base:
         # the extracted files will be in this folder
         cache_dir = Path(output_dir, _basename(filepath).split(".")[0])
@@ -337,11 +353,6 @@ def extractall(
     if cache_dir.exists() and next(cache_dir.iterdir(), None) is not None:
         logger.info(f"Non-empty folder exists in {cache_dir}, skipped extracting.")
         return
-    filepath = Path(filepath)
-    if hash_val and not check_hash(filepath, hash_val, hash_type):
-        raise RuntimeError(
-            f"{hash_type} check of compressed file failed: " f"filepath={filepath}, expected {hash_type}={hash_val}."
-        )
     logger.info(f"Writing into directory: {output_dir}.")
     _file_type = file_type.lower().strip()
     if filepath.name.endswith("zip") or _file_type == "zip":

monai/bundle/utils.py

@@ -21,7 +21,6 @@
 from monai.utils import optional_import
 
 yaml, _ = optional_import("yaml")
-
 __all__ = [
     "ID_REF_KEY",
     "ID_SEP_KEY",
@@ -39,7 +38,6 @@
 MERGE_KEY = "+"  # prefix indicating merge instead of override in case of multiple configs.
 
 _conf_values = get_config_values()
-
 DEFAULT_METADATA = {
     "version": "0.0.1",
     "changelog": {"0.0.1": "Initial version"},
@@ -211,20 +209,15 @@ def load_bundle_config(bundle_path: str, *config_names: str, **load_kw_args: Any
         name, _ = os.path.splitext(os.path.basename(bundle_path))
 
         archive = zipfile.ZipFile(bundle_path, "r")
-
         all_files = archive.namelist()
-
         zip_meta_name = f"{name}/configs/metadata.json"
-
         if zip_meta_name in all_files:
             prefix = f"{name}/configs/"  # zipped directory location for files
         else:
             zip_meta_name = f"{name}/extra/metadata.json"
             prefix = f"{name}/extra/"  # Torchscript location for files
-
         meta_json = json.loads(archive.read(zip_meta_name))
         parser.read_meta(f=meta_json)
-
         for cname in config_names:
             full_cname = prefix + cname
             if full_cname not in all_files:

monai/data/box_utils.py

@@ -1035,8 +1035,8 @@ def spatial_crop_boxes(
     # convert to float32 since torch.clamp_ does not support float16
     boxes_t = boxes_t.to(dtype=COMPUTE_DTYPE)
 
-    roi_start_t = convert_to_dst_type(src=roi_start, dst=boxes_t, wrap_sequence=True)[0].to(torch.int16)
-    roi_end_t = convert_to_dst_type(src=roi_end, dst=boxes_t, wrap_sequence=True)[0].to(torch.int16)
+    roi_start_t = convert_to_dst_type(src=roi_start, dst=boxes_t, wrap_sequence=True)[0]
+    roi_end_t = convert_to_dst_type(src=roi_end, dst=boxes_t, wrap_sequence=True)[0]
     roi_end_t = torch.maximum(roi_end_t, roi_start_t)
 
     # makes sure the bounding boxes are within the patch

monai/transforms/io/array.py

@@ -210,9 +210,7 @@ def __init__(
                 try:
                     self.register(the_reader(*args, **kwargs))
                 except OptionalImportError:
-                    warnings.warn(
-                        f"required package for reader {_r} is not installed, or the version doesn't match requirement."
-                    )
+                    raise
                 except TypeError:  # the reader doesn't have the corresponding args/kwargs
                     warnings.warn(f"{_r} is not supported with the given parameters {args} {kwargs}.")
                     self.register(the_reader())

tests/apps/test_download_and_extract.py

@@ -16,49 +16,68 @@
 import unittest
 import zipfile
 from pathlib import Path
-from urllib.error import ContentTooShortError, HTTPError
 
 from parameterized import parameterized
 
 from monai.apps import download_and_extract, download_url, extractall
+from monai.apps.utils import HashCheckError
 from tests.test_utils import SkipIfNoModule, skip_if_downloading_fails, skip_if_quick, testing_data_config
 
 
 @SkipIfNoModule("requests")
 class TestDownloadAndExtract(unittest.TestCase):
+    def setUp(self):
+        self.testing_dir = Path(__file__).parents[1] / "testing_data"
+        self.config = testing_data_config("images", "mednist")
+        self.url = self.config["url"]
+        self.hash_val = self.config["hash_val"]
+        self.hash_type = self.config["hash_type"]
+
     @skip_if_quick
-    def test_actions(self):
-        testing_dir = Path(__file__).parents[1] / "testing_data"
-        config_dict = testing_data_config("images", "mednist")
-        url = config_dict["url"]
-        filepath = Path(testing_dir) / "MedNIST.tar.gz"
-        output_dir = Path(testing_dir)
-        hash_val, hash_type = config_dict["hash_val"], config_dict["hash_type"]
+    def test_download_and_extract_success(self):
+        """End-to-end: download and extract should succeed with correct hash."""
+        filepath = self.testing_dir / "MedNIST.tar.gz"
+        output_dir = self.testing_dir
+
         with skip_if_downloading_fails():
-            download_and_extract(url, filepath, output_dir, hash_val=hash_val, hash_type=hash_type)
-            download_and_extract(url, filepath, output_dir, hash_val=hash_val, hash_type=hash_type)
+            download_and_extract(self.url, filepath, output_dir, hash_val=self.hash_val, hash_type=self.hash_type)
 
-        wrong_md5 = "0"
-        with self.assertLogs(logger="monai.apps", level="ERROR"):
-            try:
-                download_url(url, filepath, wrong_md5)
-            except (ContentTooShortError, HTTPError, RuntimeError) as e:
-                if isinstance(e, RuntimeError):
-                    # FIXME: skip MD5 check as current downloading method may fail
-                    self.assertTrue(str(e).startswith("md5 check"))
-                return  # skipping this test due the network connection errors
-
-        try:
-            extractall(filepath, output_dir, wrong_md5)
-        except RuntimeError as e:
-            self.assertTrue(str(e).startswith("md5 check"))
+        self.assertTrue(filepath.exists(), "Downloaded file does not exist")
+        self.assertTrue(any(output_dir.iterdir()), "Extraction output is empty")
+
+    @skip_if_quick
+    def test_download_url_hash_mismatch(self):
+        """download_url should raise HashCheckError on hash mismatch."""
+        filepath = self.testing_dir / "MedNIST.tar.gz"
+
+        with skip_if_downloading_fails():
+            # First ensure file is downloaded correctly
+            download_url(self.url, filepath, hash_val=self.hash_val, hash_type=self.hash_type)
+
+        # Now test incorrect hash
+        with self.assertRaises(HashCheckError):
+            download_url(self.url, filepath, hash_val="0" * len(self.hash_val), hash_type=self.hash_type)
 
     @skip_if_quick
-    @parameterized.expand((("icon", "tar"), ("favicon", "zip")))
-    def test_default(self, key, file_type):
+    def test_extractall_hash_mismatch(self):
+        """extractall should raise HashCheckError when hash is incorrect."""
+        filepath = self.testing_dir / "MedNIST.tar.gz"
+        output_dir = self.testing_dir
+
+        with skip_if_downloading_fails():
+            download_url(self.url, filepath, hash_val=self.hash_val, hash_type=self.hash_type)
+
+        with self.assertRaises(HashCheckError):
+            extractall(filepath, output_dir, hash_val="0" * len(self.hash_val), hash_type=self.hash_type)
+
+    @skip_if_quick
+    @parameterized.expand([("icon", "tar"), ("favicon", "zip")])
+    def test_download_and_extract_various_formats(self, key, file_type):
+        """Verify different archive formats download and extract correctly."""
         with tempfile.TemporaryDirectory() as tmp_dir:
+            img_spec = testing_data_config("images", key)
+
             with skip_if_downloading_fails():
-                img_spec = testing_data_config("images", key)
                 download_and_extract(
                     img_spec["url"],
                     output_dir=tmp_dir,
@@ -67,6 +86,8 @@ def test_default(self, key, file_type):
                     file_type=file_type,
                 )
 
+            self.assertTrue(any(Path(tmp_dir).iterdir()), f"Extraction failed for format: {file_type}")
+
 
 class TestPathTraversalProtection(unittest.TestCase):
     """Test cases for path traversal attack protection in extractall function."""

tests/data/test_box_utils.py

@@ -35,6 +35,7 @@
     convert_box_mode,
     convert_box_to_standard_mode,
     non_max_suppression,
+    spatial_crop_boxes,
 )
 from monai.utils.type_conversion import convert_data_type
 from tests.test_utils import TEST_NDARRAYS, assert_allclose
@@ -269,6 +270,20 @@ def test_integer_truncation_bug(self):
         self.assertTrue(np.issubdtype(iou.dtype, np.floating))
         self.assertGreater(iou[0, 0], 0.0, "IoU should not be truncated to 0")
 
+    def test_large_coordinates_are_not_dropped(self):
+        """Verify large-coordinate boxes are preserved by cropping and clipping."""
+        boxes = torch.tensor([[41000.0, 5000.0, 45000.0, 15000.0]], dtype=torch.float32)
+
+        cropped_boxes, keep = spatial_crop_boxes(
+            boxes=boxes, roi_start=[40000, 0], roi_end=[50000, 20000], remove_empty=True
+        )
+        assert_allclose(keep, torch.tensor([True]))
+        assert_allclose(cropped_boxes, torch.tensor([[1000.0, 5000.0, 5000.0, 15000.0]]))
+
+        clipped_boxes, keep = clip_boxes_to_image(boxes=boxes, spatial_size=[50000, 50000], remove_empty=True)
+        assert_allclose(keep, torch.tensor([True]))
+        assert_allclose(clipped_boxes, boxes)
+
 
 class TestBatchedNms(unittest.TestCase):
     @parameterized.expand(TEST_NDARRAYS)

tests/data/test_init_reader.py

@@ -19,7 +19,7 @@
 
 from monai.data import ITKReader, NibabelReader, NrrdReader, NumpyReader, PILReader, PydicomReader
 from monai.transforms import LoadImage, LoadImaged
-from monai.utils import MetaKeys
+from monai.utils import MetaKeys, OptionalImportError, optional_import
 from tests.test_utils import SkipIfNoModule
 
 
@@ -30,9 +30,27 @@ def test_load_image(self):
         self.assertIsInstance(instance1, LoadImage)
         self.assertIsInstance(instance2, LoadImage)
 
-        for r in ["NibabelReader", "PILReader", "ITKReader", "NumpyReader", "NrrdReader", "PydicomReader", None]:
-            inst = LoadImaged("image", reader=r)
-            self.assertIsInstance(inst, LoadImaged)
+        optional_readers = {
+            "NibabelReader": "nibabel",
+            "PILReader": "PIL",
+            "ITKReader": "itk",
+            "NrrdReader": "nrrd",
+            "PydicomReader": "pydicom",
+        }
+        for r, module in optional_readers.items():
+            with self.subTest(reader=r):
+                _, has_module = optional_import(module, allow_namespace_pkg=module in ("itk", "nrrd"))
+                if has_module:
+                    inst = LoadImaged("image", reader=r)
+                    self.assertIsInstance(inst, LoadImaged)
+                else:
+                    with self.assertRaises(OptionalImportError):
+                        LoadImaged("image", reader=r)
+
+        inst = LoadImaged("image", reader="NumpyReader")
+        self.assertIsInstance(inst, LoadImaged)
+        inst = LoadImaged("image", reader=None)
+        self.assertIsInstance(inst, LoadImaged)
 
     @SkipIfNoModule("nibabel")
     @SkipIfNoModule("cupy")

tests/test_timedcall_dist.py

@@ -19,7 +19,7 @@
 from tests.test_utils import TimedCall
 
 
-@TimedCall(seconds=20 if sys.platform == "linux" else 60, force_quit=False)
+@TimedCall(seconds=20 if sys.platform == "linux" else 60, force_quit=True)
 def case_1_seconds(arg=None):
     time.sleep(1)
     return "good" if not arg else arg