Skip to content

Commit

Permalink
remove_burnt_in, dashboard quarantine added
Browse files Browse the repository at this point in the history
  • Loading branch information
mdevans committed Oct 3, 2024
1 parent 4b8a8c7 commit d9521a4
Show file tree
Hide file tree
Showing 12 changed files with 453 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Set up Python
uses: actions/setup-python@v3
with:
python-version: "3.11"
python-version: "3.12"

- name: Install pipenv for virtual environment
run: |
Expand Down
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Release Candidate
## [17.2.1] -
### Changed
- anonymizer.py menu bar handling, resolve issue [#14](https://github.com/RSNA/anonymizer/issues/14),
- anonymizer.py menu bar handling refactored, resolve issue [#14](https://github.com/RSNA/anonymizer/issues/14),
- anonymizer.py remove menu font - * to verify on windows platforms
- Fix build.yml badge to point to correct github action build result in readme.md
- github action: build.yml change from python 3.11 to 3.12
- resolve issue [#4](https://github.com/RSNA/anonymizer/issues/4):
- model/project.py default_local_server changed IP from 127.0.0.1 to 0.0.0.0
- view/settings/dicom_node_dialog ensure "0.0.0.0" is provided to IP address dropdown

## Added
- Add tcl/tk install and test to development setup in readme.md
Expand Down
3 changes: 3 additions & 0 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ pywin32-ctypes = "*"
pefile = "*"
openpyxl = "*"
psutil = "*"
opencv-python-headless = "*"
easyocr = "*"
pylibjpeg = {extras = ["all"], version = "*"}

[dev-packages]
radon = "*"
Expand Down
2 changes: 1 addition & 1 deletion src/__version__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Major.Minor.Patch
# As per (https://semver.org/spec/v2.0.0.html)
__version__ = "17.2.1-RC1"
__version__ = "17.2.1-RC2"
103 changes: 56 additions & 47 deletions src/controller/anonymizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import os
import time
from enum import Enum
from shutil import copyfile
import re
import logging
Expand All @@ -35,6 +36,15 @@
logger = logging.getLogger(__name__)


class QuarantineDirectories(Enum):
INVALID_DICOM = _("Invalid_DICOM")
DICOM_READ_ERROR = _("DICOM_Read_Error")
MISSING_ATTRIBUTES = _("Missing_Attributes")
INVALID_STORAGE_CLASS = _("Invalid_Storage_Class")
CAPTURE_PHI_ERROR = _("Capture_PHI_Error")
STORAGE_ERROR = _("Storage_Error")


class AnonymizerController:
"""
The Anonymizer Controller class to handle the anonymization of DICOM datasets and manage the Anonymizer Model.
Expand Down Expand Up @@ -73,13 +83,6 @@ class AnonymizerController:
]

def __init__(self, project_model: ProjectModel):
# Quarantine Errors / Sub-directories:
self.QUARANTINE_INVALID_DICOM = _("Invalid_DICOM")
self.QUARANTINE_DICOM_READ_ERROR = _("DICOM_Read_Error")
self.QUARANTINE_MISSING_ATTRIBUTES = _("Missing_Attributes")
self.QUARANTINE_INVALID_STORAGE_CLASS = _("Invalid_Storage_Class")
self.QUARANTINE_CAPTURE_PHI_ERROR = _("Capture_PHI_Error")
self.QUARANTINE_STORAGE_ERROR = _("Storage_Error")
self._active = False
self.project_model = project_model
# Initialise AnonymizerModel datafile full path:
Expand Down Expand Up @@ -219,7 +222,31 @@ def local_storage_path(self, base_dir: Path, ds: Dataset) -> Path:
def get_quarantine_path(self) -> Path:
return Path(self.project_model.storage_dir, self.project_model.PRIVATE_DIR, self.project_model.QUARANTINE_DIR)

def _write_to_quarantine(self, e: Exception, ds: Dataset, quarantine_error: str) -> str:
def _move_file_to_quarantine(self, file: Path, quarantine: QuarantineDirectories) -> bool:
"""Writes the file to the specified quarantine sub-directory.
Args:
file (Path): The file to be quarantined.
sub_dir (str): The quarantine sub-directory.
Returns:
bool: True on successful move, False otherwise.
"""
try:
qpath = self.get_quarantine_path().joinpath(quarantine.value, f"{file.name}.{time.strftime('%H%M%S')}")
logger.error(f"QUARANTINE {file} to {qpath}")
if qpath.exists():
logger.error(f"File {file} already exists")
return False
os.makedirs(qpath.parent, exist_ok=True)
copyfile(file, qpath)
self.model.increment_quarantined()
return True
except Exception as e:
logger.error(f"Error Copying to QUARANTINE: {e}")
return False

def _write_dataset_to_quarantine(self, e: Exception, ds: Dataset, quarantine_error: QuarantineDirectories) -> str:
"""
Writes the given dataset to the quarantine directory and logs any errors.
Expand All @@ -231,13 +258,14 @@ def _write_to_quarantine(self, e: Exception, ds: Dataset, quarantine_error: str)
Returns:
str: The error message indicating the storage error and the path to the saved dataset.
"""
qpath: Path = self.get_quarantine_path().joinpath(quarantine_error)
qpath: Path = self.get_quarantine_path().joinpath(quarantine_error.value)
os.makedirs(qpath, exist_ok=True)
filename: Path = self.local_storage_path(qpath, ds)
try:
error_msg: str = f"Storage Error = {e}, QUARANTINE {ds.SOPInstanceUID} to {filename}"
logger.error(error_msg)
ds.save_as(filename, write_like_original=True)
self.model.increment_quarantined()
except:
logger.critical(f"Critical Error writing incoming dataset to QUARANTINE: {e}")

Expand Down Expand Up @@ -409,12 +437,9 @@ def anonymize(self, source: DICOMNode | str, ds: Dataset) -> str | None:
str | None: If an error occurs during the anonymization process, returns the error message.
Otherwise, returns None.
Raises:
LookupError: If an error occurs while capturing PHI from the dataset.
Notes:
- The anonymization process involves removing PHI from the dataset and saving the anonymized dataset to a DICOM file.
- If an error occurs during the anonymization process, the dataset is moved to the quarantine for further analysis.
- The anonymization process involves removing PHI from the dataset and saving the anonymized dataset to a DICOM file in project's storage directory.
- If an error occurs capturing PHI or storing the anonymized file, the dataset is moved to the quarantine for further analysis.
"""
self._model_change_flag = True # for autosave manager

Expand All @@ -425,9 +450,11 @@ def anonymize(self, source: DICOMNode | str, ds: Dataset) -> str | None:

# Capture PHI and source:
try:
self.model.capture_phi(str(source), ds, date_delta) # May raise LookupError
except LookupError as e:
return self._write_to_quarantine(e, ds, self.QUARANTINE_CAPTURE_PHI_ERROR)
self.model.capture_phi(str(source), ds, date_delta)
except ValueError as e:
return self._write_dataset_to_quarantine(e, ds, QuarantineDirectories.INVALID_DICOM)
except Exception as e:
return self._write_dataset_to_quarantine(e, ds, QuarantineDirectories.CAPTURE_PHI_ERROR)

phi_instance_uid = ds.SOPInstanceUID # if exception, remove this instance from uid_lookup
try:
Expand Down Expand Up @@ -475,27 +502,7 @@ def anonymize(self, source: DICOMNode | str, ds: Dataset) -> str | None:
# Remove this phi instance UID from lookup if anonymization or storage fails
# Leave other PHI intact for this patient
self.model.remove_uid(phi_instance_uid)
return self._write_to_quarantine(e, ds, self.QUARANTINE_STORAGE_ERROR)

def move_to_quarantine(self, file: Path, sub_dir: str) -> bool:
"""Writes the file to the specified quarantine sub-directory.
Args:
file (Path): The file to be quarantined.
sub_dir (str): The quarantine sub-directory.
Returns:
bool: True on successful move, False otherwise.
"""
try:
qpath = self.get_quarantine_path().joinpath(sub_dir, file.name)
logger.error(f"QUARANTINE {file} to {qpath}")
os.makedirs(qpath.parent, exist_ok=True)
copyfile(file, qpath)
return True
except Exception as e:
logger.error(f"Error Copying to QUARANTINE: {e}")
return False
return self._write_dataset_to_quarantine(e, ds, QuarantineDirectories.STORAGE_ERROR)

def anonymize_file(self, file: Path) -> tuple[str | None, Dataset | None]:
"""
Expand All @@ -516,35 +523,37 @@ def anonymize_file(self, file: Path) -> tuple[str | None, Dataset | None]:
InvalidDicomError: If the DICOM file is invalid.
Exception: If any other unexpected exception occurs during the anonymization process.
"""
self._model_change_flag = True # for autosave manager

try:
ds: Dataset = dcmread(file)
except (FileNotFoundError, IsADirectoryError, PermissionError) as e:
logger.error(str(e))
return str(e), None
except InvalidDicomError as e:
self.move_to_quarantine(file, self.QUARANTINE_INVALID_DICOM)
return str(e), None
self._move_file_to_quarantine(file, QuarantineDirectories.INVALID_DICOM)
return str(e) + " -> " + _("Quarantined"), None
except Exception as e:
self.move_to_quarantine(file, self.QUARANTINE_DICOM_READ_ERROR)
return str(e), None
self._move_file_to_quarantine(file, QuarantineDirectories.DICOM_READ_ERROR)
return str(e) + " -> " + _("Quarantined"), None

# DICOM Dataset integrity checking:
missing_attributes: list[str] = self.missing_attributes(ds)
if missing_attributes != []:
self.move_to_quarantine(file, self.QUARANTINE_MISSING_ATTRIBUTES)
return f"Missing Attributes: {missing_attributes}", ds
self._move_file_to_quarantine(file, QuarantineDirectories.MISSING_ATTRIBUTES)
return _("Missing Attributes") + f": {missing_attributes}" + " -> " + _("Quarantined"), ds

# Skip instance if already stored:
if self.model.get_anon_uid(ds.SOPInstanceUID):
logger.info(
f"Instance already stored:{ds.PatientID}/{ds.StudyInstanceUID}/{ds.SeriesInstanceUID}/{ds.SOPInstanceUID}"
)
return (f"Instance already stored", ds)
return (_("Instance already stored"), ds)

# Ensure Storage Class (SOPClassUID which is a required attribute) is present in project storage classes
if ds.SOPClassUID not in self.project_model.storage_classes:
self.move_to_quarantine(file, self.QUARANTINE_INVALID_STORAGE_CLASS)
return f"Storage Class: {ds.SOPClassUID} mismatch", ds
self._move_file_to_quarantine(file, QuarantineDirectories.INVALID_STORAGE_CLASS)
return _("Storage Class mismatch") + f": {ds.SOPClassUID}" + " -> " + _("Quarantined"), ds

return self.anonymize(str(file), ds), ds

Expand Down
Loading

0 comments on commit d9521a4

Please sign in to comment.