Update SDK to support typeguard v3.0 and above, which has a breaking change in its API, and updated notebooks (#438)

* Change to support typeguard v3.0+ which has breaking API change on check_type()

Along with the SDK change in th io_context.py, any pinged version of typarguard to ~=2
must be removed.

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Add comments on the reason for depending on pydicom and highdicom

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Add comments in the notebooks for the use of highdicom package

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Reran notebooks, aferwards, restored import "--upgrade"

With "import --upgrade monai-deploy-app-sdk", users do not need to explicitly uninstall
prior version of the App SDK when running the notebook in virtual env. For testing the
notebook in dev with the editable App SDK, do NOT use --upgrade in case the local dirty
version is lower somehow.

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Udpate pytest version to get the automated test in CI to succeed

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Bump to Python3.8 as in 3.7 pytest still fails on GitHub

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Fix flake8 complaint: B907, manually surrounded by quotes, consider using the `!r` conversion

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Quiet the format checking complaint

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* To quiet flake8 complint

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Yet another black nit-picking

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Need to use the new way, pytest instead of py.test

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Somehow the last commit did not get the change py.test -> pytest

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Retested all notebooks as QA step

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Yet another way to quiet flake8 complaint

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* The file content was wiped out (local disk space issue)

Signed-off-by: M Q <mingmelvinq@nvidia.com>

* Used the known working way to quiet flake8 complaint, str.format()

Signed-off-by: M Q <mingmelvinq@nvidia.com>

---------

Signed-off-by: M Q <mingmelvinq@nvidia.com>

Author: MMelQin

.readthedocs.yml

@@ -18,7 +18,7 @@ sphinx:
 
 # Optionally set the version of Python and requirements required to build your docs
 python:
-  version: 3.7
+  version: "3.8"
   install:
     - requirements: docs/requirements.txt
 #  system_packages: true

examples/apps/mednist_classifier_monaideploy/mednist_classifier_monaideploy.py

@@ -95,7 +95,7 @@ def compute(self, op_input: InputContext, op_output: OutputContext, context: Exe
 
 
 @md.resource(cpu=1, gpu=1, memory="1Gi")
-@md.env(pip_packages=["pydicom >= 2.3.0", "highdicom>=0.18.2", "typeguard~=2.12.1"])
+@md.env(pip_packages=["pydicom >= 2.3.0", "highdicom>=0.18.2"])  # because of the use of DICOM writer operator
 class App(Application):
     """Application class for the MedNIST classifier."""
 

monai/deploy/core/application.py

@@ -205,13 +205,13 @@ def add_flow(
         if not io_map:
             if len(op_output_labels) > 1:
                 raise IOMappingError(
-                    f"The source operator has more than one output port "
+                    "The source operator has more than one output port "
                     f"({', '.join(op_output_labels)}) so mapping should be specified explicitly!"
                 )
             if len(op_input_labels) > 1:
                 raise IOMappingError(
                     f"The destination operator has more than one output port ({', '.join(op_input_labels)}) "
-                    f"so mapping should be specified explicitly!"
+                    "so mapping should be specified explicitly!"
                 )
             io_map = {"": {""}}
 
@@ -227,7 +227,7 @@ def add_flow(
         if len(op_output_labels) == 1 and len(output_labels) != 1:
             raise IOMappingError(
                 f"The source operator({source_op.name}) has only one port with label "
-                f"'{next(iter(op_output_labels))}' but io_map specifies {len(output_labels)} "
+                f"{next(iter(op_output_labels))!r} but io_map specifies {len(output_labels)} "
                 f"labels({', '.join(output_labels)}) to the source operator's output port"
             )
 
@@ -239,7 +239,7 @@ def add_flow(
                     del io_maps[output_label]
                     break
                 raise IOMappingError(
-                    f"The source operator({source_op.name}) has no output port with label '{output_label}'. "
+                    f"The source operator({source_op.name}) has no output port with label {output_label!r}. "
                     f"It should be one of ({', '.join(op_output_labels)})."
                 )
 
@@ -250,7 +250,7 @@ def add_flow(
             if len(op_input_labels) == 1 and len(input_labels) != 1:
                 raise IOMappingError(
                     f"The destination operator({destination_op.name}) has only one port with label "
-                    f"'{next(iter(op_input_labels))}' but io_map specifies {len(input_labels)} "
+                    f"{next(iter(op_input_labels))!r} but io_map specifies {len(input_labels)} "
                     f"labels({', '.join(input_labels)}) to the destination operator's input port"
                 )
 
@@ -262,8 +262,8 @@ def add_flow(
                         input_labels.add(next(iter(op_input_labels)))
                         break
                     raise IOMappingError(
-                        f"The destination operator({destination_op.name}) has no input port with label '{input_label}'. "
-                        f"It should be one of ({', '.join(op_input_labels)})."
+                        f"The destination operator({destination_op.name}) has no input port with label {input_label!r}."
+                        f" It should be one of ({', '.join(op_input_labels)})."
                     )
 
         self._graph.add_flow(source_op, destination_op, io_maps)

monai/deploy/core/domain/datapath.py

@@ -67,10 +67,10 @@ def get(self, name: Optional[str] = "") -> DataPath:
                 return next(iter(self._paths.values()))
             else:
                 raise IOMappingError(
-                    f"'{name}' is not a valid name. It should be one of ({', '.join(self._paths.keys())})."
+                    f"{name!r} is not a valid name. It should be one of ({', '.join(self._paths.keys())})."
                 )
         else:
             datapath = self._paths.get(name)
             if not datapath:
-                raise ItemNotExistsError(f"A DataPath instance for '{name}' does not exist.")
+                raise ItemNotExistsError(f"A DataPath instance for {name!r} does not exist.")
             return datapath

monai/deploy/core/env.py

@@ -37,7 +37,7 @@ def __init__(self, pip_packages: Optional[Union[str, List[str]]] = None):
             if requirements_path.exists():
                 pip_packages = requirements_path.read_text().strip().splitlines()  # make it a list
             else:
-                raise FileNotFoundError(f"The '{requirements_path}' file does not exist!")
+                raise FileNotFoundError(f"The {requirements_path!r} file does not exist!")
 
         self._pip_packages = list(pip_packages or [])
 

monai/deploy/core/io_context.py

@@ -49,7 +49,7 @@ def get_default_label(self, label: str = "") -> str:
                 label = next(iter(self._labels))
             else:
                 raise IOMappingError(
-                    f"'{label}' is not a valid {self._io_kind} of the operator({self._op.name}). "
+                    f"{label!r} is not a valid {self._io_kind} of the operator({self._op.name}). "
                     f"It should be one of ({', '.join(self._labels)})."
                 )
         return label
@@ -82,7 +82,7 @@ def get(self, label: str = "") -> Any:
         key = self.get_group_path(f"{self._io_kind}/{label}")
         storage = self._storage
         if not storage.exists(key):
-            raise ItemNotExistsError(f"'{key}' does not exist.")
+            raise ItemNotExistsError(f"{key!r} does not exist.")
         return storage.get(key)
 
     def set(self, value: Any, label: str = ""):
@@ -109,10 +109,10 @@ def set(self, value: Any, label: str = ""):
             # checking: https://www.python.org/dev/peps/pep-0585/#id15
             data_type = self._op_info.get_data_type(self._io_kind, label)
             try:
-                check_type("value", value, data_type)
+                check_type(value, data_type)
             except TypeError as err:
                 raise IOMappingError(
-                    f"The data type of '{label}' in the {self._io_kind} of '{self._op}' is {data_type}, but the value"
+                    f"The data type of {label!r} in the {self._io_kind} of {self._op!r} is {data_type}, but the value"
                     f" to set is the data type of {type(value)}."
                 ) from err
 

monai/deploy/core/models/model.py

@@ -164,7 +164,7 @@ def get(self, name: str = "") -> "Model":
             if item:
                 return item
             else:
-                raise ItemNotExistsError(f"A model with '{name}' does not exist.")
+                raise ItemNotExistsError(f"A model with {name!r} does not exist.")
         else:
             item_count = len(self._items)
             if item_count == 1:

monai/deploy/core/models/named_model.py

@@ -83,7 +83,7 @@ def accept(cls, path: str):
             # 3) Each model folder must contain only one model definition file or folder.
             if sum(1 for _ in model_folder.iterdir()) != 1:
                 logger.warning(
-                    f"Model repository '{model_folder}' contains more than one model definition file or folder "
+                    f"Model repository {model_folder!r} contains more than one model definition file or folder "
                     "so not treated as NamedModel."
                 )
                 return False, None

monai/deploy/core/resource.py

@@ -64,15 +64,17 @@ def set_resource_limits(
                 self._cpu = cpu_limit
             else:
                 raise ItemAlreadyExistsError(
-                    f"'cpu' wouldn't be set to {cpu_limit} because it is already set to {self._cpu} by the runtime environment."
+                    f"'cpu' wouldn't be set to {cpu_limit} because it is already set to {self._cpu} by the runtime"
+                    " environment."
                 )
 
         if gpu_limit is not None:
             if self._gpu is None:
                 self._gpu = gpu_limit
             else:
                 raise ItemAlreadyExistsError(
-                    f"'gpu' wouldn't be set to {gpu_limit} because it is already set to {self._gpu} by the runtime environment."
+                    f"'gpu' wouldn't be set to {gpu_limit} because it is already set to {self._gpu} by the runtime"
+                    " environment."
                 )
 
         if type(memory_limit) == str:

monai/deploy/operators/dicom_data_loader_operator.py

@@ -308,7 +308,8 @@ def test():
                 print(f"   'StudyInstanceUID': {ds.StudyInstanceUID if ds.StudyInstanceUID else ''}")
                 print(f"  'SeriesDescription': {ds.SeriesDescription if ds.SeriesDescription else ''}")
                 print(
-                    f"  'IssuerOfPatientID': {ds.get('IssuerOfPatientID', '').repval if ds.get('IssuerOfPatientID', '') else '' }"
+                    "  'IssuerOfPatientID':"
+                    f" {ds.get('IssuerOfPatientID', '').repval if ds.get('IssuerOfPatientID', '') else '' }"
                 )
                 try:
                     print(f"  'IssuerOfPatientID': {ds.IssuerOfPatientID if ds.IssuerOfPatientID else '' }")

monai/deploy/operators/dicom_series_selector_operator.py

@@ -177,7 +177,7 @@ def _select_all_series(self, dicom_study_list: List[DICOMStudy]) -> List[StudySe
         study_selected_series_list = []
         for study in dicom_study_list:
             logging.info(f"Working on study, instance UID: {study.StudyInstanceUID}")
-            print((f"Working on study, instance UID: {study.StudyInstanceUID}"))
+            print(f"Working on study, instance UID: {study.StudyInstanceUID}")
             study_selected_series = StudySelectedSeries(study)
             for series in study.get_all_series():
                 logging.info(f"Working on series, instance UID: {str(series.SeriesInstanceUID)}")
@@ -213,7 +213,7 @@ def _select_series(self, attributes: dict, study: DICOMStudy, all_matched=False)
             matched = True
             # Simple matching on attribute value
             for key, value_to_match in attributes.items():
-                logging.info(f"On attribute: '{key}' to match value: '{value_to_match}'")
+                logging.info(f"On attribute: {key!r} to match value: {value_to_match!r}")
                 # Ignore None
                 if not value_to_match:
                     continue

monai/deploy/operators/monai_bundle_inference_operator.py

@@ -80,7 +80,7 @@ def _read_from_archive(archive, root_name: str, config_name: str, do_search=True
         for suffix in bundle_suffixes:
             path = Path(root_name, config_folder, config_name).with_suffix(suffix)
             try:
-                logging.debug(f"Trying to read config '{config_name}' content from {path}.")
+                logging.debug(f"Trying to read config {config_name!r} content from {path}.")
                 content_text = archive.read(str(path))
                 break
             except Exception:
@@ -89,12 +89,12 @@ def _read_from_archive(archive, root_name: str, config_name: str, do_search=True
 
         # Try search for the name in the name list of the archive
         if not content_text and do_search:
-            logging.debug(f"Trying to find the file in the archive for config '{config_name}'.")
+            logging.debug(f"Trying to find the file in the archive for config {config_name!r}.")
             name_list = archive.namelist()
             for suffix in bundle_suffixes:
                 for n in name_list:
                     if (f"{config_name}{suffix}").casefold in n.casefold():
-                        logging.debug(f"Trying to read content of config '{config_name}' from {n}.")
+                        logging.debug(f"Trying to read content of config {config_name!r} from {n}.")
                         content_text = archive.read(n)
                         break
 

monai/deploy/packager/util.py

@@ -93,11 +93,9 @@ def initialize_args(args: Namespace) -> Dict:
         dockerfile_type = verify_base_image(args.base)
         if not dockerfile_type:
             logger.error(
-                "Provided base image '{}' is not supported \n \
-                        Please provide a ROCm or Cuda based Pytorch image from \n \
-                        https://hub.docker.com/r/rocm/pytorch or https://ngc.nvidia.com/ (nvcr.io/nvidia)".format(
-                    args.base
-                )
+                "Provided base image '{}' is not supported \n                         Please provide a ROCm or Cuda"
+                " based Pytorch image from \n                         https://hub.docker.com/r/rocm/pytorch or"
+                " https://ngc.nvidia.com/ (nvcr.io/nvidia)".format(args.base)
             )
 
             sys.exit(1)
@@ -237,7 +235,7 @@ def build_image(args: dict, temp_dir: str):
         dockerignore_file.write(docker_ignore_template)
 
     # Build dockerfile into an MAP image
-    docker_build_cmd = f'''docker build -f "{docker_file_path}" -t {tag} "{temp_dir}"'''
+    docker_build_cmd = f"""docker build -f {docker_file_path!r} -t {tag} {temp_dir!r}"""
     if sys.platform != "win32":
         docker_build_cmd += """ --build-arg MONAI_UID=$(id -u) --build-arg MONAI_GID=$(id -g)"""
     if no_cache:

monai/deploy/runner/runner.py

@@ -41,7 +41,7 @@ def fetch_map_manifest(map_name: str) -> Tuple[dict, dict, int]:
 
     with tempfile.TemporaryDirectory() as info_dir:
         if sys.platform == "win32":
-            cmd = f'docker run --rm -a STDOUT -a STDERR -v "{info_dir}":/var/run/monai/export/config {map_name}'
+            cmd = f'docker run --rm -a STDOUT -a STDERR -v " {info_dir}":/var/run/monai/export/config {map_name}'
         else:
             cmd = f"""docker_id=$(docker create {map_name})
 docker cp $docker_id:/etc/monai/app.json "{info_dir}/app.json"
@@ -103,8 +103,8 @@ def run_app(map_name: str, input_path: Path, output_path: Path, app_info: dict,
     if not posixpath.isabs(map_output):
         map_output = posixpath.join(app_info["working-directory"], map_output)
 
-    cmd += f' -e MONAI_INPUTPATH="{map_input}"'
-    cmd += f' -e MONAI_OUTPUTPATH="{map_output}"'
+    cmd += ' -e MONAI_INPUTPATH="{}"'.format(map_input)
+    cmd += ' -e MONAI_OUTPUTPATH="{}"'.format(map_output)
     # TODO(bhatt-piyush): Handle model environment correctly (maybe solved by fixing 'monai-exec')
     cmd += " -e MONAI_MODELPATH=/opt/monai/models"
 

monai/deploy/utils/argparse_types.py

@@ -36,7 +36,7 @@ def valid_dir_path(path: str) -> Path:
         if dir_path.is_dir():
             return dir_path
         else:
-            raise argparse.ArgumentTypeError(f"Expected directory path: '{dir_path}' is not a directory")
+            raise argparse.ArgumentTypeError(f"Expected directory path: {dir_path!r} is not a directory")
 
     # create directory
     dir_path.mkdir(parents=True)
@@ -58,7 +58,7 @@ def valid_existing_dir_path(path: str) -> Path:
     dir_path = Path(path).absolute()
     if dir_path.exists() and dir_path.is_dir():
         return dir_path
-    raise argparse.ArgumentTypeError(f"No such directory: '{dir_path}'")
+    raise argparse.ArgumentTypeError(f"No such directory: {dir_path!r}")
 
 
 def valid_existing_path(path: str) -> Path:
@@ -76,4 +76,4 @@ def valid_existing_path(path: str) -> Path:
     file_path = Path(path).absolute()
     if file_path.exists():
         return file_path
-    raise argparse.ArgumentTypeError(f"No such file/folder: '{file_path}'")
+    raise argparse.ArgumentTypeError(f"No such file/folder: {file_path!r}")

monai/deploy/utils/importutil.py

@@ -238,7 +238,7 @@ def optional_import(
     # preparing lazy error message
     msg = descriptor.format(actual_cmd)
     if version and tb is None:  # a pure version issue
-        msg += f" (requires '{module} {version}' by '{version_checker.__name__}')"
+        msg += f" (requires '{module} {version}' by {version_checker.__name__!r})"
     if exception_str:
         msg += f" ({exception_str})"
 

monai/deploy/utils/sizeutil.py

@@ -66,7 +66,7 @@ def get_bytes(size: Union[str, int]) -> int:
 
     m = re.match(r"^\s*(?P<size>(([1-9]\d+)|\d)(\.\d+)?)\s*(?P<unit>[a-z]{1,3})?\s*$", size, re.IGNORECASE)
     if not m:
-        raise ValueError(f"Invalid size string ('{size}').")
+        raise ValueError(f"Invalid size string ({size!r}).")
 
     parsed_size = float(m.group("size"))
 
@@ -77,7 +77,7 @@ def get_bytes(size: Union[str, int]) -> int:
         parsed_unit = "b"  # default to bytes
 
     if parsed_unit not in BYTES_UNIT:
-        raise ValueError(f"Invalid unit ('{parsed_unit}').")
+        raise ValueError(f"Invalid unit ({parsed_unit!r}).")
 
     return int(parsed_size * BYTES_UNIT[parsed_unit])
 
@@ -115,7 +115,7 @@ def convert_bytes(num_bytes: int, unit: str = "Mi") -> Union[str, int]:
 
     unit_lowered = unit.lower()
     if unit_lowered not in BYTES_UNIT:
-        raise ValueError(f"Invalid unit ('{unit}').")
+        raise ValueError(f"Invalid unit ({unit!r}).")
 
     converted = float(num_bytes) / BYTES_UNIT[unit_lowered] * 10
 

monai/deploy/utils/version.py

@@ -123,7 +123,7 @@ def get_sdk_semver():
         if SEMVER_REGEX.match(semver_str):
             return semver_str
         else:
-            raise ValueError(f"Invalid semver string: '{semver_str}' (from '{version_str}')")
+            raise ValueError(f"Invalid semver string: {semver_str!r} (from {version_str!r})")
     else:
         raise ValueError(f"Invalid version string: {version_str}")