From c65066cf137e6f79a943505c1c5349c78b03493e Mon Sep 17 00:00:00 2001 From: Gigon Bae Date: Fri, 10 Sep 2021 19:10:24 -0700 Subject: [PATCH 1/2] Fix all type errors in existing code Signed-off-by: Gigon Bae --- .../ai_spleen_seg_app/spleen_seg_operator.py | 1 + monai/deploy/cli/__main__.py | 2 +- monai/deploy/core/domain/dicom_series.py | 83 ++++++++++++------- .../deploy/core/domain/dicom_sop_instance.py | 21 +++-- monai/deploy/core/domain/dicom_study.py | 28 ++++--- monai/deploy/core/domain/domain.py | 2 +- .../operators/dicom_data_loader_operator.py | 7 +- .../operators/dicom_seg_writer_operator.py | 34 ++++---- .../operators/monai_seg_inference_operator.py | 31 ++++--- monai/deploy/packager/util.py | 10 +-- setup.cfg | 6 +- tests/system/packager/test_packager.py | 2 +- tests/unit/test_argparse_types.py | 48 +++++------ 13 files changed, 158 insertions(+), 117 deletions(-) diff --git a/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py b/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py index 2665769d..e1be0f8e 100644 --- a/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py +++ b/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py @@ -12,6 +12,7 @@ import logging from monai.deploy.core import ExecutionContext, Image, InputContext, IOType, Operator, OutputContext, env, input, output + from monai.deploy.operators.monai_seg_inference_operator import InMemImageReader, MonaiSegInferenceOperator from monai.transforms import ( Activationsd, diff --git a/monai/deploy/cli/__main__.py b/monai/deploy/cli/__main__.py index 1fcbe7c6..d1152497 100644 --- a/monai/deploy/cli/__main__.py +++ b/monai/deploy/cli/__main__.py @@ -10,7 +10,7 @@ # limitations under the License. -from main import main +from main import main # type: ignore # for pytype if __name__ == "__main__": main() diff --git a/monai/deploy/core/domain/dicom_series.py b/monai/deploy/core/domain/dicom_series.py index e303782b..fab607c8 100644 --- a/monai/deploy/core/domain/dicom_series.py +++ b/monai/deploy/core/domain/dicom_series.py @@ -9,6 +9,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import Any + from .dicom_sop_instance import DICOMSOPInstance from .domain import Domain @@ -21,6 +23,23 @@ def __init__(self, series_instance_uid): self._series_instance_uid = series_instance_uid self._sop_instances = [] + self._series_date: Any = None + self._series_time: Any = None + self._modality: Any = None + self._series_description: Any = None + self._body_part_examined: Any = None + self._patient_position: Any = None + self._series_number: Any = None + self._laterality: Any = None + self._row_pixel_spacing: Any = None + self._col_pixel_spacing: Any = None + self._depth_pixel_spacing: Any = None + self._row_direction_cosine: Any = None + self._col_direction_cosine: Any = None + self._depth_direction_cosine: Any = None + self._dicom_affine_transform: Any = None + self._nifti_affine_transform: Any = None + def get_series_instance_uid(self): return self._series_instance_uid @@ -33,131 +52,131 @@ def get_sop_instances(self): @property def series_date(self): - return self.__series_date + return self._series_date @series_date.setter def series_date(self, val): - self.__series_date = val + self._series_date = val @property def series_time(self): - return self.__series_time + return self._series_time @series_time.setter def series_time(self, val): - self.__series_time = val + self._series_time = val @property def modality(self): - return self.__modality + return self._modality @modality.setter def modality(self, val): - self.__modality = val + self._modality = val @property def series_description(self): - return self.__series_description + return self._series_description @series_description.setter def series_description(self, val): - self.__series_description = val + self._series_description = val @property def body_part_examined(self): - return self.__body_part_examined + return self._body_part_examined @body_part_examined.setter def body_part_examined(self, val): - self.__body_part_examined = val + self._body_part_examined = val @property def patient_position(self): - return self.__patient_position + return self._patient_position @patient_position.setter def patient_position(self, val): - self.__patient_position = val + self._patient_position = val @property def series_number(self): - return self.__series_number + return self._series_number @series_number.setter def series_number(self, val): - self.__series_number = val + self._series_number = val @property def laterality(self): - return self.__laterality + return self._laterality @laterality.setter def laterality(self, val): - self.__laterality = val + self._laterality = val @property def row_pixel_spacing(self): - return self.__row_pixel_spacing + return self._row_pixel_spacing @row_pixel_spacing.setter def row_pixel_spacing(self, val): - self.__row_pixel_spacing = val + self._row_pixel_spacing = val @property def col_pixel_spacing(self): - return self.__col_pixel_spacing + return self._col_pixel_spacing @col_pixel_spacing.setter def col_pixel_spacing(self, val): - self.__col_pixel_spacing = val + self._col_pixel_spacing = val @property def depth_pixel_spacing(self): - return self.__depth_pixel_spacing + return self._depth_pixel_spacing @depth_pixel_spacing.setter def depth_pixel_spacing(self, val): - self.__depth_pixel_spacing = val + self._depth_pixel_spacing = val @property def row_direction_cosine(self): - return self.__row_direction_cosine + return self._row_direction_cosine @row_direction_cosine.setter def row_direction_cosine(self, val): - self.__row_direction_cosine = val + self._row_direction_cosine = val @property def col_direction_cosine(self): - return self.__col_direction_cosine + return self._col_direction_cosine @col_direction_cosine.setter def col_direction_cosine(self, val): - self.__col_direction_cosine = val + self._col_direction_cosine = val @property def depth_direction_cosine(self): - return self.__depth_direction_cosine + return self._depth_direction_cosine @depth_direction_cosine.setter def depth_direction_cosine(self, val): - self.__depth_direction_cosine = val + self._depth_direction_cosine = val @property def dicom_affine_transform(self): - return self.__dicom_affine_transform + return self._dicom_affine_transform @dicom_affine_transform.setter def dicom_affine_transform(self, val): - self.__dicom_affine_transform = val + self._dicom_affine_transform = val @property def nifti_affine_transform(self): - return self.__nifti_affine_transform + return self._nifti_affine_transform @nifti_affine_transform.setter def nifti_affine_transform(self, val): - self.__nifti_affine_transform = val + self._nifti_affine_transform = val def __str__(self): result = "---------------" + "\n" diff --git a/monai/deploy/core/domain/dicom_sop_instance.py b/monai/deploy/core/domain/dicom_sop_instance.py index 69964a0f..6dca1b0b 100644 --- a/monai/deploy/core/domain/dicom_sop_instance.py +++ b/monai/deploy/core/domain/dicom_sop_instance.py @@ -9,18 +9,21 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Union +from typing import Any, Union from monai.deploy.utils.importutil import optional_import from .domain import Domain -DataElement, _ = optional_import("pydicom", name="DataElement") -Dataset, _ = optional_import("pydicom", name="Dataset") -Tag, _ = optional_import("pydicom.tag", name="Tag") -BaseTag, _ = optional_import("pydicom.tag", name="BaseTag") -tag_in_exception, _ = optional_import("pydicom.tag", name="tag_in_exception") -TagType, _ = optional_import("pydicom.tag", name="TagType") +DataElement_, _ = optional_import("pydicom", name="DataElement") +# Dynamic class is not handled so make it Any for now: https://github.com/python/mypy/issues/2477 +DataElement: Any = DataElement_ +Dataset_, _ = optional_import("pydicom", name="Dataset") +# Dynamic class is not handled so make it Any for now: https://github.com/python/mypy/issues/2477 +Dataset: Any = Dataset_ +TagType_, _ = optional_import("pydicom.tag", name="TagType") +# Dynamic class is not handled so make it Any for now: https://github.com/python/mypy/issues/2477 +TagType: Any = TagType_ class DICOMSOPInstance(Domain): @@ -31,12 +34,12 @@ class DICOMSOPInstance(Domain): def __init__(self, native_sop): super().__init__(None) - self._sop = native_sop + self._sop: Any = native_sop def get_native_sop_instance(self): return self._sop - def __getitem__(self, key: Union[int, slice, "TagType"]) -> Union["Dataset", "DataElement"]: + def __getitem__(self, key: Union[int, slice, TagType]) -> Union[Dataset, DataElement]: return self._sop.__getitem__(key) def get_pixel_array(self): diff --git a/monai/deploy/core/domain/dicom_study.py b/monai/deploy/core/domain/dicom_study.py index eda07e54..91944469 100644 --- a/monai/deploy/core/domain/dicom_study.py +++ b/monai/deploy/core/domain/dicom_study.py @@ -10,6 +10,8 @@ # limitations under the License. +from typing import Any + from .domain import Domain @@ -24,6 +26,12 @@ def __init__(self, study_instance_uid): self._study_instance_uid = study_instance_uid self._series_dict = {} + self._study_id: Any = None + self._study_date: Any = None + self._study_time: Any = None + self._study_description: Any = None + self._accession_number: Any = None + def get_study_instance_uid(self): return self._study_instance_uid @@ -35,43 +43,43 @@ def get_all_series(self): @property def study_id(self): - return self.__study_id + return self._study_id @study_id.setter def study_id(self, val): - self.__study_id = val + self._study_id = val @property def study_date(self): - return self.__study_date + return self._study_date @study_date.setter def study_date(self, val): - self.__study_date = val + self._study_date = val @property def study_time(self): - return self.__study_time + return self._study_time @study_time.setter def study_time(self, val): - self.__study_time = val + self._study_time = val @property def study_description(self): - return self.__study_description + return self._study_description @study_description.setter def study_description(self, val): - self.__study_description = val + self._study_description = val @property def accession_number(self): - return self.__accession_number + return self._accession_number @accession_number.setter def accession_number(self, val): - self.__accession_number = val + self._accession_number = val def __str__(self): result = "---------------" + "\n" diff --git a/monai/deploy/core/domain/domain.py b/monai/deploy/core/domain/domain.py index c4babf17..eaff44af 100644 --- a/monai/deploy/core/domain/domain.py +++ b/monai/deploy/core/domain/domain.py @@ -29,5 +29,5 @@ def __init__(self, metadata: Optional[Dict] = None): else: self._metadata = {} - def metadata(self) -> Optional[Dict]: + def metadata(self) -> Dict: return self._metadata diff --git a/monai/deploy/operators/dicom_data_loader_operator.py b/monai/deploy/operators/dicom_data_loader_operator.py index 5b24dc52..a6b40b1c 100644 --- a/monai/deploy/operators/dicom_data_loader_operator.py +++ b/monai/deploy/operators/dicom_data_loader_operator.py @@ -10,6 +10,7 @@ # limitations under the License. import os +from typing import List from monai.deploy.core import ( DataPath, @@ -48,13 +49,13 @@ def compute(self, input: InputContext, output: OutputContext, context: Execution It groups them by a collection of studies where each study contains one or more series. This method returns a set of studies. """ - files = [] + files: List[str] = [] input_path = input.get().path self._list_files(input_path, files) dicom_study_list = self._load_data(files) output.set(dicom_study_list) - def _list_files(self, path, files): + def _list_files(self, path, files: List[str]): """Collects fully qualified names of all files recurvisely given a directory path. Args: @@ -68,7 +69,7 @@ def _list_files(self, path, files): else: files.append(item) - def _load_data(self, files): + def _load_data(self, files: List[str]): """Provides a list of DICOM Studies given a list of fully qualified file names. Args: diff --git a/monai/deploy/operators/dicom_seg_writer_operator.py b/monai/deploy/operators/dicom_seg_writer_operator.py index 7ec6014c..797a873b 100644 --- a/monai/deploy/operators/dicom_seg_writer_operator.py +++ b/monai/deploy/operators/dicom_seg_writer_operator.py @@ -15,7 +15,7 @@ import logging import os from random import randint -from typing import List, Union +from typing import List, Optional, Union import numpy as np @@ -63,7 +63,7 @@ class DICOMSegmentationWriterOperator(Operator): # Suffix to add to file name to indicate DICOM Seg dcm file. DICOMSEG_SUFFIX = "-DICOMSEG" - def __init__(self, seg_labels: Union[List[str], str] = None, *args, **kwargs): + def __init__(self, seg_labels: Optional[Union[List[str], str]] = None, *args, **kwargs): super().__init__(*args, **kwargs) """Instantiates the DICOM Seg Writer instance with optional list of segment label strings. @@ -82,7 +82,7 @@ def __init__(self, seg_labels: Union[List[str], str] = None, *args, **kwargs): segment label information, including label value, name, description etc. Args: - seg_labels (List[str] or str): The string name for each segment + seg_labels: The string name for each segment """ self._seg_labels = ["SegmentLabel-default"] @@ -94,7 +94,7 @@ def __init__(self, seg_labels: Union[List[str], str] = None, *args, **kwargs): raise ValueError(f"List of strings expected, but contains {label} of type {type(label)}.") self._seg_labels = seg_labels else: - raise ValueError(f"List of strings expected.") + raise ValueError("List of strings expected.") def compute(self, input: InputContext, output: OutputContext, context: ExecutionContext): dicom_series = input.get("dicom_series") @@ -370,7 +370,7 @@ def create_multiframe_metadata(dicom_file, input_ds): dicomOutput.add_new(0x00080013, "TM", currentTime) # InstanceCreationTime # General Image module. - dicomOutput.add_new(0x00080008, "CS", ["DERIVED", "PRIMARY"]) # ImageType + dicomOutput.add_new(0x00080008, "CS", ["DERIVED", "PRIMARY"]) # ImageType dicomOutput.add_new(0x00200020, "CS", "") # PatientOrientation, forced empty # Set content date/time dicomOutput.ContentDate = currentDate @@ -415,7 +415,7 @@ def create_label_segment(label, name): segment.add_new(0x00620009, "LO", "AI Organ Segmentation") # SegmentAlgorithmName segment.SegmentAlgorithmType = "AUTOMATIC" # SegmentAlgorithmType segment.add_new(0x0062000D, "US", [128, 174, 128]) # RecommendedDisplayCIELabValue - ## create SegmentedPropertyCategoryCodeSequence + # Create SegmentedPropertyCategoryCodeSequence segmentedPropertyCategoryCodeSequence = Sequence() segmentedPropertyCategoryCodeSequenceDS = Dataset() segmentedPropertyCategoryCodeSequenceDS.add_new(0x00080100, "SH", "T-D0050") # CodeValue @@ -423,7 +423,7 @@ def create_label_segment(label, name): segmentedPropertyCategoryCodeSequenceDS.add_new(0x00080104, "LO", "Anatomical Structure") # CodeMeaning segmentedPropertyCategoryCodeSequence.append(segmentedPropertyCategoryCodeSequenceDS) segment.SegmentedPropertyCategoryCodeSequence = segmentedPropertyCategoryCodeSequence - ## create SegmentedPropertyTypeCodeSequence + # Create SegmentedPropertyTypeCodeSequence segmentedPropertyTypeCodeSequence = Sequence() segmentedPropertyTypeCodeSequenceDS = Dataset() segmentedPropertyTypeCodeSequenceDS.add_new(0x00080100, "SH", "T-D0050") # CodeValue @@ -458,17 +458,17 @@ def create_frame_meta(input_ds, label, ref_instances, dimIdxVal, instance_num): ############################ # CREATE METADATA ############################ - ##### create DerivationImageSequence within Per-frame Functional Groups sequence + # Create DerivationImageSequence within Per-frame Functional Groups sequence derivationImageSequence = Sequence() derivationImage = Dataset() - ### create SourceImageSequence within DerivationImageSequence + # Create SourceImageSequence within DerivationImageSequence sourceImageSequence = Sequence() sourceImage = Dataset() # TODO if CT multi-frame # sourceImage.add_new(0x00081160, 'IS', inputFrameCounter + 1) # Referenced Frame Number sourceImage.add_new(0x00081150, "UI", sourceInstanceSOPClass) # ReferencedSOPClassUID sourceImage.add_new(0x00081155, "UI", sop_inst_uid) # ReferencedSOPInstanceUID - ## create PurposeOfReferenceCodeSequence within SourceImageSequence + # Create PurposeOfReferenceCodeSequence within SourceImageSequence purposeOfReferenceCodeSequence = Sequence() purposeOfReferenceCode = Dataset() purposeOfReferenceCode.add_new(0x00080100, "SH", "121322") # CodeValue @@ -477,7 +477,7 @@ def create_frame_meta(input_ds, label, ref_instances, dimIdxVal, instance_num): purposeOfReferenceCodeSequence.append(purposeOfReferenceCode) sourceImage.add_new(0x0040A170, "SQ", purposeOfReferenceCodeSequence) # PurposeOfReferenceCodeSequence sourceImageSequence.append(sourceImage) # AEH Beck commentout - ### create DerivationCodeSequence within DerivationImageSequence + # Create DerivationCodeSequence within DerivationImageSequence derivationCodeSequence = Sequence() derivationCode = Dataset() derivationCode.add_new(0x00080100, "SH", "113076") # CodeValue @@ -488,25 +488,25 @@ def create_frame_meta(input_ds, label, ref_instances, dimIdxVal, instance_num): derivationImage.add_new(0x00082112, "SQ", sourceImageSequence) # SourceImageSequence derivationImageSequence.append(derivationImage) frame_ds.add_new(0x00089124, "SQ", derivationImageSequence) # DerivationImageSequence - ##### create FrameContentSequence within Per-frame Functional Groups sequence + # Create FrameContentSequence within Per-frame Functional Groups sequence frameContent = Sequence() dimensionIndexValues = Dataset() dimensionIndexValues.add_new(0x00209157, "UL", [dimIdxVal, instance_num]) # DimensionIndexValues frameContent.append(dimensionIndexValues) frame_ds.add_new(0x00209111, "SQ", frameContent) # FrameContentSequence - ##### create PlanePositionSequence within Per-frame Functional Groups sequence + # Create PlanePositionSequence within Per-frame Functional Groups sequence planePositionSequence = Sequence() imagePositionPatient = Dataset() imagePositionPatient.add_new(0x00200032, "DS", safe_get(input_ds, 0x00200032)) # ImagePositionPatient planePositionSequence.append(imagePositionPatient) frame_ds.add_new(0x00209113, "SQ", planePositionSequence) # PlanePositionSequence - ##### create PlaneOrientationSequence within Per-frame Functional Groups sequence + # Create PlaneOrientationSequence within Per-frame Functional Groups sequence planeOrientationSequence = Sequence() imageOrientationPatient = Dataset() imageOrientationPatient.add_new(0x00200037, "DS", safe_get(input_ds, 0x00200037)) # ImageOrientationPatient planeOrientationSequence.append(imageOrientationPatient) frame_ds.add_new(0x00209116, "SQ", planeOrientationSequence) # PlaneOrientationSequence - ##### create SegmentIdentificationSequence within Per-frame Functional Groups sequence + # Create SegmentIdentificationSequence within Per-frame Functional Groups sequence segmentIdentificationSequence = Sequence() referencedSegmentNumber = Dataset() # TODO lop over label and only get pixel with that value @@ -527,7 +527,7 @@ def set_pixel_meta(dicomOutput, input_ds): dicomOutput.PixelRepresentation = 0 # dicomOutput.PixelRepresentation = input_ds.PixelRepresentation dicomOutput.SamplesPerPixel = 1 - dicomOutput.ImageType = "DERIVED\PRIMARY" + dicomOutput.ImageType = "DERIVED\\PRIMARY" dicomOutput.ContentLabel = "SEGMENTATION" dicomOutput.ContentDescription = "" dicomOutput.ContentCreatorName = "" @@ -598,7 +598,7 @@ def segslice_from_mhd(dcm_output, seg_img, input_ds, num_labels): dcm_output.get(0x00081115)[0].add_new(0x0008114A, "SQ", referenceInstances) # ReferencedInstanceSequence - ##### create shared Functional Groups sequence + # Create shared Functional Groups sequence sharedFunctionalGroups = Sequence() sharedFunctionalGroupsDS = Dataset() diff --git a/monai/deploy/operators/monai_seg_inference_operator.py b/monai/deploy/operators/monai_seg_inference_operator.py index 864b158b..73dcd5a8 100644 --- a/monai/deploy/operators/monai_seg_inference_operator.py +++ b/monai/deploy/operators/monai_seg_inference_operator.py @@ -20,11 +20,15 @@ np_str_obj_array_pattern, _ = optional_import("torch.utils.data._utils.collate", name="np_str_obj_array_pattern") Dataset, _ = optional_import("monai.data", name="Dataset") DataLoader, _ = optional_import("monai.data", name="DataLoader") -ImageReader, _ = optional_import("monai.data", name="ImageReader") +ImageReader_, _ = optional_import("monai.data", name="ImageReader") +# Dynamic class is not handled so make it Any for now: https://github.com/python/mypy/issues/2477 +ImageReader: Any = ImageReader_ decollate_batch, _ = optional_import("monai.data", name="decollate_batch") sliding_window_inference, _ = optional_import("monai.inferers", name="sliding_window_inference") ensure_tuple, _ = optional_import("monai.utils", name="ensure_tuple") -Compose, _ = optional_import("monai.transforms", name="Compose") +Compose_, _ = optional_import("monai.transforms", name="Compose") +# Dynamic class is not handled so make it Any for now: https://github.com/python/mypy/issues/2477 +Compose: Any = Compose_ sliding_window_inference, _ = optional_import("monai.inferers", name="sliding_window_inference") from monai.deploy.core import ExecutionContext, Image, InputContext, IOType, OutputContext, env, input, output @@ -52,7 +56,12 @@ class MonaiSegInferenceOperator(InferenceOperator): MODEL_LOCAL_PATH = "model/model.ts" def __init__( - self, roi_size: Union[Sequence[int], int], pre_transforms: Compose, post_transforms: Compose, *args, **kwargs + self, + roi_size: Union[Sequence[int], int], + pre_transforms: Compose, + post_transforms: Compose, + *args, + **kwargs, ): """Creates a instance of this class. @@ -68,7 +77,7 @@ def __init__( self._input_dataset_key = "image" self._pred_dataset_key = "pred" self._input_image = None # Image will come in when compute is called. - self._reader = None + self._reader: Any = None self._roi_size = ensure_tuple(roi_size) self._pre_transform = pre_transforms self._post_transforms = post_transforms @@ -128,8 +137,8 @@ def compute(self, input: InputContext, output: OutputContext, context: Execution except Exception: # Best effort pass - pre_transforms = self._pre_transform - post_transforms = self._post_transforms + pre_transforms: Compose = self._pre_transform + post_transforms: Compose = self._post_transforms self._reader = InMemImageReader(input_image) pre_transforms = self._pre_transform if self._pre_transform else self.pre_process(self._reader) @@ -148,13 +157,13 @@ def compute(self, input: InputContext, output: OutputContext, context: Execution # If model_name is not specified and only one model exists, it returns that model. model = context.models.get() print(f"Model path: {model.path}") - print(f"Model name (expected None): {model.name}") + print(f"Model name: {model.name}") else: print(f"Loading TorchScript model from: {MonaiSegInferenceOperator.MODEL_LOCAL_PATH}") model = torch.jit.load(MonaiSegInferenceOperator.MODEL_LOCAL_PATH) dataset = Dataset(data=[{self._input_dataset_key: img_name}], transform=pre_transforms) - dataloader = DataLoader(dataset, batch_size=1, shuffle=False, num_workers=4) + dataloader = DataLoader(dataset, batch_size=1, shuffle=False, num_workers=1) device = torch.device("cuda" if torch.cuda.is_available() else "cpu") with torch.no_grad(): @@ -190,7 +199,7 @@ def compute(self, input: InputContext, output: OutputContext, context: Execution with self._lock: self._executing = False - def pre_process(self, img_reader) -> Compose: + def pre_process(self, img_reader) -> Union[Any, Image, Compose]: """Transforms input before being used for predicting on a model. This method must be overridden by a derived class. @@ -200,7 +209,7 @@ def pre_process(self, img_reader) -> Compose: """ raise NotImplementedError(f"Subclass {self.__class__.__name__} must implement this method.") - def post_process(self, pre_transforms: Compose, out_dir: str = "./infer_out") -> Compose: + def post_process(self, pre_transforms: Compose, out_dir: str = "./infer_out") -> Union[Any, Image, Compose]: """Transform the prediction results from the model(s). This method must be overridden by a derived class. @@ -269,7 +278,7 @@ def _get_meta_dict(self, img: Image) -> Dict: Args: img: a MONAI Deploy App SDK Image object. """ - img_meta_dict = img.metadata() + img_meta_dict: Dict = img.metadata() meta_dict = {key: img_meta_dict[key] for key in img_meta_dict.keys()} # Will have to derive some key metadata as the SDK Image lacks the necessary interfaces. diff --git a/monai/deploy/packager/util.py b/monai/deploy/packager/util.py index 873b1033..144b32c8 100644 --- a/monai/deploy/packager/util.py +++ b/monai/deploy/packager/util.py @@ -98,13 +98,12 @@ def initialize_args(args: Namespace) -> Dict: processed_args["dockerfile_type"] = dockerfile_type if args.base else DefaultValues.DOCKERFILE_TYPE - base_image = "" + base_image = DefaultValues.BASE_IMAGE if args.base: base_image = args.base - elif os.getenv("MONAI_BASEIMAGE"): - base_image = os.getenv("MONAI_BASEIMAGE") else: - base_image = DefaultValues.BASE_IMAGE + base_image = os.getenv("MONAI_BASEIMAGE", DefaultValues.BASE_IMAGE) + processed_args["base_image"] = base_image # Obtain SDK provide application values @@ -239,7 +238,8 @@ def build_image(args: dict, temp_dir: str): spinner.start() while proc.poll() is None: - logger.debug(proc.stdout.readline().decode("utf-8")) + if proc.stdout: + logger.debug(proc.stdout.readline().decode("utf-8")) spinner.stop() return_code = proc.returncode diff --git a/setup.cfg b/setup.cfg index 067c9e79..fb40e65e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -42,7 +42,11 @@ ignore = E203,E305,E402,E501,E721,E741,F821,F841,F999,W503,W504,C408,E302,W291,E303, # N812 lowercase 'torch.nn.functional' imported as non lowercase 'F' N812 -per_file_ignores = __init__.py: F401 +per_file_ignores = + __init__.py: F401 + # Allow using camel case for variable/argument names for the sake of readability. + # 43 warnings with N803(2) N806(41) in dicom_seg_writer_operator.py + dicom_seg_writer_operator.py: N803, N806 exclude = *.pyi,.git,.eggs,monai/deploy/_version.py,versioneer.py,venv,.venv,_version.py [isort] diff --git a/tests/system/packager/test_packager.py b/tests/system/packager/test_packager.py index ef15957e..e04cb4f5 100644 --- a/tests/system/packager/test_packager.py +++ b/tests/system/packager/test_packager.py @@ -23,5 +23,5 @@ def test_packager(): monai_deploy(args) # Delete MONAI application package image - docker_rmi_cmd= ["docker", "rmi", "-f", test_map_tag] + docker_rmi_cmd = ["docker", "rmi", "-f", test_map_tag] subprocess.Popen(docker_rmi_cmd) diff --git a/tests/unit/test_argparse_types.py b/tests/unit/test_argparse_types.py index d76a7ae2..ff737b05 100644 --- a/tests/unit/test_argparse_types.py +++ b/tests/unit/test_argparse_types.py @@ -9,17 +9,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +import getpass +from argparse import ArgumentTypeError from contextlib import contextmanager -from os import path -from unittest.mock import patch from pathlib import Path, PosixPath import pytest -import getpass from pytest_lazyfixture import lazy_fixture -from argparse import ArgumentTypeError - @pytest.mark.parametrize("expected_dir_path", [lazy_fixture("tmp_path"), lazy_fixture("non_existent_file_path")]) def test_valid_dir_path_valid_args(expected_dir_path): @@ -29,9 +26,9 @@ def test_valid_dir_path_valid_args(expected_dir_path): assert type(actual_dir_path) == PosixPath assert actual_dir_path == expected_dir_path - assert expected_dir_path.exists() == True - assert actual_dir_path.is_dir() == True - assert actual_dir_path.is_absolute() == True + assert expected_dir_path.exists() is True + assert actual_dir_path.is_dir() is True + assert actual_dir_path.is_absolute() is True assert actual_dir_path.owner() == getpass.getuser() @@ -57,9 +54,9 @@ def working_directory(path): assert type(actual_dir_path) == PosixPath assert actual_dir_path == expected_dir_path - assert expected_dir_path.exists() == True - assert actual_dir_path.is_dir() == True - assert actual_dir_path.is_absolute() == True + assert expected_dir_path.exists() is True + assert actual_dir_path.is_dir() is True + assert actual_dir_path.is_absolute() is True assert actual_dir_path.owner() == getpass.getuser() @@ -67,29 +64,29 @@ def test_valid_dir_path_invalid_args(faux_file): from monai.deploy.utils.argparse_types import valid_dir_path expected_invalid_dir_path = faux_file - assert expected_invalid_dir_path.exists() == True - assert expected_invalid_dir_path.is_dir() == False + assert expected_invalid_dir_path.exists() is True + assert expected_invalid_dir_path.is_dir() is False with pytest.raises(ArgumentTypeError) as wrapped_error: valid_dir_path(str(expected_invalid_dir_path)) assert wrapped_error.type == ArgumentTypeError - assert expected_invalid_dir_path.exists() == True - assert expected_invalid_dir_path.is_dir() == False + assert expected_invalid_dir_path.exists() is True + assert expected_invalid_dir_path.is_dir() is False def test_valid_existing_dir_path_valid_args(tmp_path): from monai.deploy.utils.argparse_types import valid_existing_dir_path expected_dir_path = tmp_path - assert expected_dir_path.exists() == True - assert expected_dir_path.is_dir() == True + assert expected_dir_path.exists() is True + assert expected_dir_path.is_dir() is True actual_dir_path = valid_existing_dir_path(str(expected_dir_path)) assert type(actual_dir_path) == PosixPath assert actual_dir_path == expected_dir_path - assert expected_dir_path.exists() == True - assert actual_dir_path.is_dir() == True + assert expected_dir_path.exists() is True + assert actual_dir_path.is_dir() is True assert actual_dir_path.owner() == getpass.getuser() @@ -97,19 +94,19 @@ def test_valid_existing_dir_path_valid_args(tmp_path): def test_valid_existing_dir_path_invalid_args(input_path): from monai.deploy.utils.argparse_types import valid_existing_dir_path - assert input_path.is_dir() == False + assert input_path.is_dir() is False with pytest.raises(ArgumentTypeError) as wrapped_error: valid_existing_dir_path(str(input_path)) assert wrapped_error.type == ArgumentTypeError - assert input_path.is_dir() == False + assert input_path.is_dir() is False @pytest.mark.parametrize("expected_input_path", [lazy_fixture("faux_file"), lazy_fixture("faux_folder")]) def test_valid_existing_path_valid_args(expected_input_path): from monai.deploy.utils.argparse_types import valid_existing_path - assert expected_input_path.exists() == True + assert expected_input_path.exists() is True actual_input_path = valid_existing_path(str(expected_input_path)) assert type(actual_input_path) == PosixPath @@ -118,13 +115,12 @@ def test_valid_existing_path_valid_args(expected_input_path): @pytest.mark.parametrize("input_path", [lazy_fixture("non_existent_file_path")]) -def test_valid_existing_path_valid_args(input_path): +def test_valid_existing_path_invalid_args(input_path): from monai.deploy.utils.argparse_types import valid_existing_path - assert input_path.exists() == False + assert input_path.exists() is False with pytest.raises(ArgumentTypeError) as wrapped_error: valid_existing_path(str(input_path)) assert wrapped_error.type == ArgumentTypeError - assert input_path.exists() == False - + assert input_path.exists() is False From b0ad0eec55f6a4ea0b835c1bc07f9feb673d9f7d Mon Sep 17 00:00:00 2001 From: Gigon Bae Date: Sat, 11 Sep 2021 04:09:47 -0700 Subject: [PATCH 2/2] Fix DICOM attribute handling Existing implementation did not set attributes in advance, and their availability are checked by handing AttributeError. For the reason, initializing those attributes with None caused spleen app execution failure. (All metadata dictionary in Domain class is filled with None even for non-available attributes) With this patch, the attribute's availability is checked by checking if its value is None and getters return None if attributes are not set (using getattr() method). Signed-off-by: Gigon Bae --- .../ai_spleen_seg_app/spleen_seg_operator.py | 1 - monai/deploy/core/domain/dicom_series.py | 123 +++++++----------- monai/deploy/core/domain/dicom_study.py | 59 ++++----- .../dicom_series_to_volume_operator.py | 48 ++----- 4 files changed, 90 insertions(+), 141 deletions(-) diff --git a/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py b/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py index e1be0f8e..2665769d 100644 --- a/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py +++ b/examples/apps/ai_spleen_seg_app/spleen_seg_operator.py @@ -12,7 +12,6 @@ import logging from monai.deploy.core import ExecutionContext, Image, InputContext, IOType, Operator, OutputContext, env, input, output - from monai.deploy.operators.monai_seg_inference_operator import InMemImageReader, MonaiSegInferenceOperator from monai.transforms import ( Activationsd, diff --git a/monai/deploy/core/domain/dicom_series.py b/monai/deploy/core/domain/dicom_series.py index fab607c8..01fcb7a2 100644 --- a/monai/deploy/core/domain/dicom_series.py +++ b/monai/deploy/core/domain/dicom_series.py @@ -9,8 +9,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any - from .dicom_sop_instance import DICOMSOPInstance from .domain import Domain @@ -23,22 +21,24 @@ def __init__(self, series_instance_uid): self._series_instance_uid = series_instance_uid self._sop_instances = [] - self._series_date: Any = None - self._series_time: Any = None - self._modality: Any = None - self._series_description: Any = None - self._body_part_examined: Any = None - self._patient_position: Any = None - self._series_number: Any = None - self._laterality: Any = None - self._row_pixel_spacing: Any = None - self._col_pixel_spacing: Any = None - self._depth_pixel_spacing: Any = None - self._row_direction_cosine: Any = None - self._col_direction_cosine: Any = None - self._depth_direction_cosine: Any = None - self._dicom_affine_transform: Any = None - self._nifti_affine_transform: Any = None + # Do not set attributes in advance to save memory + + # self._series_date: Any = None + # self._series_time: Any = None + # self._modality: Any = None + # self._series_description: Any = None + # self._body_part_examined: Any = None + # self._patient_position: Any = None + # self._series_number: Any = None + # self._laterality: Any = None + # self._row_pixel_spacing: Any = None + # self._col_pixel_spacing: Any = None + # self._depth_pixel_spacing: Any = None + # self._row_direction_cosine: Any = None + # self._col_direction_cosine: Any = None + # self._depth_direction_cosine: Any = None + # self._dicom_affine_transform: Any = None + # self._nifti_affine_transform: Any = None def get_series_instance_uid(self): return self._series_instance_uid @@ -52,7 +52,7 @@ def get_sop_instances(self): @property def series_date(self): - return self._series_date + return getattr(self, "_series_date", None) @series_date.setter def series_date(self, val): @@ -60,7 +60,7 @@ def series_date(self, val): @property def series_time(self): - return self._series_time + return getattr(self, "_series_time", None) @series_time.setter def series_time(self, val): @@ -68,7 +68,7 @@ def series_time(self, val): @property def modality(self): - return self._modality + return getattr(self, "_modality", None) @modality.setter def modality(self, val): @@ -76,7 +76,7 @@ def modality(self, val): @property def series_description(self): - return self._series_description + return getattr(self, "_series_description", None) @series_description.setter def series_description(self, val): @@ -84,7 +84,7 @@ def series_description(self, val): @property def body_part_examined(self): - return self._body_part_examined + return getattr(self, "_body_part_examined", None) @body_part_examined.setter def body_part_examined(self, val): @@ -92,7 +92,7 @@ def body_part_examined(self, val): @property def patient_position(self): - return self._patient_position + return getattr(self, "_patient_position", None) @patient_position.setter def patient_position(self, val): @@ -100,7 +100,7 @@ def patient_position(self, val): @property def series_number(self): - return self._series_number + return getattr(self, "_series_number", None) @series_number.setter def series_number(self, val): @@ -108,7 +108,7 @@ def series_number(self, val): @property def laterality(self): - return self._laterality + return getattr(self, "_laterality", None) @laterality.setter def laterality(self, val): @@ -116,7 +116,7 @@ def laterality(self, val): @property def row_pixel_spacing(self): - return self._row_pixel_spacing + return getattr(self, "_row_pixel_spacing", None) @row_pixel_spacing.setter def row_pixel_spacing(self, val): @@ -124,7 +124,7 @@ def row_pixel_spacing(self, val): @property def col_pixel_spacing(self): - return self._col_pixel_spacing + return getattr(self, "_col_pixel_spacing", None) @col_pixel_spacing.setter def col_pixel_spacing(self, val): @@ -132,7 +132,7 @@ def col_pixel_spacing(self, val): @property def depth_pixel_spacing(self): - return self._depth_pixel_spacing + return getattr(self, "_depth_pixel_spacing", None) @depth_pixel_spacing.setter def depth_pixel_spacing(self, val): @@ -140,7 +140,7 @@ def depth_pixel_spacing(self, val): @property def row_direction_cosine(self): - return self._row_direction_cosine + return getattr(self, "_row_direction_cosine", None) @row_direction_cosine.setter def row_direction_cosine(self, val): @@ -148,7 +148,7 @@ def row_direction_cosine(self, val): @property def col_direction_cosine(self): - return self._col_direction_cosine + return getattr(self, "_col_direction_cosine", None) @col_direction_cosine.setter def col_direction_cosine(self, val): @@ -156,7 +156,7 @@ def col_direction_cosine(self, val): @property def depth_direction_cosine(self): - return self._depth_direction_cosine + return getattr(self, "_depth_direction_cosine", None) @depth_direction_cosine.setter def depth_direction_cosine(self, val): @@ -164,7 +164,7 @@ def depth_direction_cosine(self, val): @property def dicom_affine_transform(self): - return self._dicom_affine_transform + return getattr(self, "_dicom_affine_transform", None) @dicom_affine_transform.setter def dicom_affine_transform(self, val): @@ -172,7 +172,7 @@ def dicom_affine_transform(self, val): @property def nifti_affine_transform(self): - return self._nifti_affine_transform + return getattr(self, "_nifti_affine_transform", None) @nifti_affine_transform.setter def nifti_affine_transform(self, val): @@ -184,83 +184,56 @@ def __str__(self): series_instance_uid_attr = "Series Instance UID: " + self._series_instance_uid + "\n" result += series_instance_uid_attr - try: - num_sop_instances = "Num SOP Instances: " + str(len(self._sop_instances)) + "\n" - result += num_sop_instances - except AttributeError: - pass + num_sop_instances = "Num SOP Instances: " + str(len(self._sop_instances)) + "\n" + result += num_sop_instances - try: + if self.series_date is not None: series_date_attr = "Series Date: " + self.series_date + "\n" result += series_date_attr - except AttributeError: - pass - try: + if self.series_time is not None: series_time_attr = "Series Time: " + self.series_time + "\n" result += series_time_attr - except AttributeError: - pass - try: + if self.modality is not None: modality_attr = "Modality: " + self.modality + "\n" result += modality_attr - except AttributeError: - pass - try: + if self.series_description is not None: series_desc_attr = "Series Description: " + self.series_description + "\n" result += series_desc_attr - except AttributeError: - pass - try: + if self.row_pixel_spacing is not None: row_pixel_spacing_attr = "Row Pixel Spacing: " + str(self.row_pixel_spacing) + "\n" result += row_pixel_spacing_attr - except AttributeError: - pass - try: + if self.col_pixel_spacing is not None: col_pixel_spacing_attr = "Column Pixel Spacing: " + str(self.col_pixel_spacing) + "\n" result += col_pixel_spacing_attr - except AttributeError: - pass - try: + if self.depth_pixel_spacing is not None: depth_pixel_spacing_attr = "Depth Pixel Spacing: " + str(self.depth_pixel_spacing) + "\n" result += depth_pixel_spacing_attr - except AttributeError: - pass - try: + if self.row_direction_cosine is not None: row_direction_cosine_attr = "Row Direction Cosine: " + str(self.row_direction_cosine) + "\n" result += row_direction_cosine_attr - except AttributeError: - pass - try: + if self.col_direction_cosine is not None: col_direction_cosine_attr = "Column Direction Cosine: " + str(self.col_direction_cosine) + "\n" result += col_direction_cosine_attr - except AttributeError: - pass - try: + if self.depth_direction_cosine is not None: depth_direction_cosine_attr = "Depth Direction Cosine: " + str(self.depth_direction_cosine) + "\n" result += depth_direction_cosine_attr - except AttributeError: - pass - try: + if self.dicom_affine_transform is not None: dicom_affine_transform_attr = "DICOM affine transform: " + "\n" + str(self.dicom_affine_transform) + "\n" result += dicom_affine_transform_attr - except AttributeError: - pass - try: + if self.nifti_affine_transform is not None: nifti_affine_transform_attr = "NIFTI affine transform: " + "\n" + str(self.nifti_affine_transform) + "\n" result += nifti_affine_transform_attr - except AttributeError: - pass result += "---------------" + "\n" diff --git a/monai/deploy/core/domain/dicom_study.py b/monai/deploy/core/domain/dicom_study.py index 91944469..b7a1c0dc 100644 --- a/monai/deploy/core/domain/dicom_study.py +++ b/monai/deploy/core/domain/dicom_study.py @@ -10,8 +10,6 @@ # limitations under the License. -from typing import Any - from .domain import Domain @@ -26,11 +24,13 @@ def __init__(self, study_instance_uid): self._study_instance_uid = study_instance_uid self._series_dict = {} - self._study_id: Any = None - self._study_date: Any = None - self._study_time: Any = None - self._study_description: Any = None - self._accession_number: Any = None + # Do not set attributes in advance to save memory + + # self._study_id: Any = None + # self._study_date: Any = None + # self._study_time: Any = None + # self._study_description: Any = None + # self._accession_number: Any = None def get_study_instance_uid(self): return self._study_instance_uid @@ -43,7 +43,7 @@ def get_all_series(self): @property def study_id(self): - return self._study_id + return getattr(self, "_study_id", None) @study_id.setter def study_id(self, val): @@ -51,7 +51,7 @@ def study_id(self, val): @property def study_date(self): - return self._study_date + return getattr(self, "_study_date", None) @study_date.setter def study_date(self, val): @@ -59,7 +59,7 @@ def study_date(self, val): @property def study_time(self): - return self._study_time + return getattr(self, "_study_time", None) @study_time.setter def study_time(self, val): @@ -67,7 +67,7 @@ def study_time(self, val): @property def study_description(self): - return self._study_description + return getattr(self, "_study_description", None) @study_description.setter def study_description(self, val): @@ -75,7 +75,7 @@ def study_description(self, val): @property def accession_number(self): - return self._accession_number + return getattr(self, "_accession_number", None) @accession_number.setter def accession_number(self, val): @@ -84,23 +84,24 @@ def accession_number(self, val): def __str__(self): result = "---------------" + "\n" - study_instance_uid_attr = "Study Instance UID: " + self._study_instance_uid + "\n" - result += study_instance_uid_attr - - study_id_attr = "Study ID: " + self.study_id + "\n" - result += study_id_attr - - study_date_attr = "Study Date: " + self.study_date + "\n" - result += study_date_attr - - study_time_attr = "Study Time: " + self.study_time + "\n" - result += study_time_attr - - study_desc_attr = "Study Description: " + self.study_description + "\n" - result += study_desc_attr - - accession_num_attr = "Accession Number: " + self.accession_number + "\n" - result += accession_num_attr + if self._study_instance_uid is not None: + study_instance_uid_attr = "Study Instance UID: " + self._study_instance_uid + "\n" + result += study_instance_uid_attr + if self.study_id is not None: + study_id_attr = "Study ID: " + self.study_id + "\n" + result += study_id_attr + if self.study_date is not None: + study_date_attr = "Study Date: " + self.study_date + "\n" + result += study_date_attr + if self.study_time is not None: + study_time_attr = "Study Time: " + self.study_time + "\n" + result += study_time_attr + if self.study_description is not None: + study_desc_attr = "Study Description: " + self.study_description + "\n" + result += study_desc_attr + if self.accession_number is not None: + accession_num_attr = "Accession Number: " + self.accession_number + "\n" + result += accession_num_attr result += "---------------" + "\n" diff --git a/monai/deploy/operators/dicom_series_to_volume_operator.py b/monai/deploy/operators/dicom_series_to_volume_operator.py index 6241ca5e..67e05acf 100644 --- a/monai/deploy/operators/dicom_series_to_volume_operator.py +++ b/monai/deploy/operators/dicom_series_to_volume_operator.py @@ -287,65 +287,41 @@ def create_metadata(self, series): metadata = {} metadata["series_instance_uid"] = series.get_series_instance_uid() - try: + if series.series_date is not None: metadata["series_date"] = series.series_date - except AttributeError: - pass - try: + if series.series_time is not None: metadata["series_time"] = series.series_time - except AttributeError: - pass - try: + if series.modality is not None: metadata["modality"] = series.modality - except AttributeError: - pass - try: + if series.series_description is not None: metadata["series_description"] = series.series_description - except AttributeError: - pass - try: + if series.row_pixel_spacing is not None: metadata["row_pixel_spacing"] = series.row_pixel_spacing - except AttributeError: - pass - try: + if series.col_pixel_spacing is not None: metadata["col_pixel_spacing"] = series.col_pixel_spacing - except AttributeError: - pass - try: + if series.depth_pixel_spacing is not None: metadata["depth_pixel_spacing"] = series.depth_pixel_spacing - except AttributeError: - pass - try: + if series.row_direction_cosine is not None: metadata["row_direction_cosine"] = series.row_direction_cosine - except AttributeError: - pass - try: + if series.col_direction_cosine is not None: metadata["col_direction_cosine"] = series.col_direction_cosine - except AttributeError: - pass - try: + if series.depth_direction_cosine is not None: metadata["depth_direction_cosine"] = series.depth_direction_cosine - except AttributeError: - pass - try: + if series.dicom_affine_transform is not None: metadata["dicom_affine_transform"] = series.dicom_affine_transform - except AttributeError: - pass - try: + if series.nifti_affine_transform is not None: metadata["nifti_affine_transform"] = series.nifti_affine_transform - except AttributeError: - pass return metadata