From c97b3fbe285d3c7b3cd8d2d872eeb410af0a5206 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos Orfanos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sun, 15 Sep 2024 19:58:23 +0200 Subject: [PATCH 1/3] Enforce repo-review rules (#296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Enforce repo-review rule PP302 PP302: Sets a minimum pytest to at least 6 Must have a `minversion=`, and must be at least 6 (first version to support `pyproject.toml` configuration). * Enforce repo-review rule PP305 PP305: Specifies xfail_strict `xfail_strict` should be set. You can manually specify if a check should be strict when setting each xfail. * Enforce repo-review rule PP306 PP306: Specifies strict config `--strict-config` should be in `addopts = [...]`. This forces an error if a config setting is misspelled. * Enforce repo-review rule PP307 PP307: Specifies strict markers `--strict-markers` should be in `addopts = [...]`. This forces all markers to be specified in config, avoiding misspellings. * Enforce repo-review rule PP308 PP308: Specifies useful pytest summary An explicit summary flag like `-ra` should be in `addopts = [...]` (print summary of all fails/errors). * Enforce pytest ≥ 7.3.2 This is the first version to support Python 3.12: https://docs.pytest.org/en/stable/changelog.html#pytest-7-3-2-2023-06-10 * Enforce repo-review rule MY104 MY104: MyPy enables ignore-without-code Must have `"ignore-without-code"` in `enable_error_code = [...]`. This will force all skips in your project to include the error code, which makes them more readable, and avoids skipping something unintended. * Enforce repo-review rule MY105 MY105: MyPy enables redundant-expr Must have `"redundant-expr"` in `enable_error_code = [...]`. This helps catch useless lines of code, like checking the same condition twice. * Enforce repo-review rule MY106 MY106: MyPy enables truthy-bool Must have `"truthy-bool"` in `enable_error_code = []`. This catches mistakes in using a value as truthy if it cannot be falsy. --- pyproject.toml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4bb80de4..185e2f26 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,8 +48,8 @@ libjpeg = [ ] test = [ "mypy==0.971", - "pytest==7.1.2", - "pytest-cov==3.0.0", + "pytest==7.4.4", + "pytest-cov==4.1.0", "pytest-flake8==1.1.1", "numpy-stubs @ git+https://github.com/numpy/numpy-stubs@201115370a0c011d879d69068b60509accc7f750", ] @@ -67,12 +67,15 @@ documentation = "https://highdicom.readthedocs.io/" repository = "https://github.com/ImagingDataCommons/highdicom.git" [tool.pytest.ini_options] -addopts = "--doctest-modules" +minversion = "7" +addopts = ["--doctest-modules", "-ra", "--strict-config", "--strict-markers"] testpaths = ["tests"] log_cli_level = "INFO" +xfail_strict = true [tool.mypy] warn_unreachable = true +enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] [[tool.mypy.overrides]] module = "mypy-pydicom.*" From cef570cb4b4d92a724ea0421e158d4e91896489a Mon Sep 17 00:00:00 2001 From: Chris Bridge Date: Mon, 23 Sep 2024 19:31:41 -0400 Subject: [PATCH 2/3] Add get_evidence and get_evidence_series methods for _SR --- src/highdicom/sr/sop.py | 108 ++++++++++++++++++++++++++++++++++++ tests/test_sr.py | 119 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+) diff --git a/src/highdicom/sr/sop.py b/src/highdicom/sr/sop.py index 44ed97a2..b13cd952 100644 --- a/src/highdicom/sr/sop.py +++ b/src/highdicom/sr/sop.py @@ -1,11 +1,13 @@ """Module for SOP Classes of Structured Report (SR) IODs.""" import datetime +from itertools import chain import logging from collections import defaultdict from copy import deepcopy from os import PathLike from typing import ( Any, + Generator, cast, Mapping, List, @@ -351,6 +353,112 @@ def content(self) -> ContentSequence: """highdicom.sr.value_types.ContentSequence: SR document content""" return self._content + def get_evidence( + self, + current_procedure_only: bool = False, + ) -> List[Tuple[UID, UID, UID, UID]]: + """Get a list of all SOP Instances listed as evidence in this SR. + + Parameters + ---------- + current_procedure_only: bool, optional + If True, return only evidence created in order to satisfy the + current requested procedure (found in the + *CurrentRequestedProcedureEvidenceSequence*). If False, also + include other evidence (found in the + *PertinentOtherEvidenceSequence*). + + Returns + ------- + List[Tuple[highdicom.UID, highdicom.UID, highdicom.UID, highdicom.UID]]: + List of tuples of UIDs, each representing a single instance. Each + tuple consists of (StudyInstanceUID, SeriesInstanceUID, + SOPInstanceUID, SOPClassUID). + + """ + def extract_evidence( + sequence: DataElementSequence, + ) -> Generator[Tuple[UID, UID, UID, UID], None, None]: + for item in sequence: + for series_ds in item.ReferencedSeriesSequence: + for instance_ds in series_ds.ReferencedSOPSequence: + yield ( + UID(item.StudyInstanceUID), + UID(series_ds.SeriesInstanceUID), + UID(instance_ds.ReferencedSOPInstanceUID), + UID(instance_ds.ReferencedSOPClassUID), + ) + + current_evidence_seq = self.get('CurrentRequestedProcedureEvidenceSequence') + if current_evidence_seq is not None: + current_evidence = extract_evidence(current_evidence_seq) + else: + current_evidence = [] + + other_evidence_seq = self.get('PertinentOtherEvidenceSequence') + if other_evidence_seq is not None and not current_procedure_only: + other_evidence = extract_evidence(other_evidence_seq) + else: + other_evidence = [] + + evidence = list(chain(current_evidence, other_evidence)) + + # Deduplicate the list while preserving order + evidence = list(dict.fromkeys(evidence)) + + return evidence + + def get_evidence_series( + self, + current_procedure_only: bool = False, + ) -> List[Tuple[UID, UID]]: + """Get a list of all series listed as evidence in this SR. + + Parameters + ---------- + current_procedure_only: bool, optional + If True, return only evidence created in order to satisfy the + current requested procedure (found in the + *CurrentRequestedProcedureEvidenceSequence*). If False, also + include other evidence (found in the + *PertinentOtherEvidenceSequence*). + + Returns + ------- + List[Tuple[highdicom.UID, highdicom.UID]]: + List of tuples of UIDs, each representing a single series. Each + tuple consists of (StudyInstanceUID, SeriesInstanceUID, + SOPInstanceUID, SOPClassUID). + + """ + def extract_evidence_series( + sequence: DataElementSequence, + ) -> Generator[Tuple[UID, UID], None, None]: + for item in sequence: + for series_ds in item.ReferencedSeriesSequence: + yield ( + UID(item.StudyInstanceUID), + UID(series_ds.SeriesInstanceUID), + ) + + current_evidence_seq = self.get('CurrentRequestedProcedureEvidenceSequence') + if current_evidence_seq is not None: + current_evidence = extract_evidence_series(current_evidence_seq) + else: + current_evidence = [] + + other_evidence_seq = self.get('PertinentOtherEvidenceSequence') + if other_evidence_seq is not None and not current_procedure_only: + other_evidence = extract_evidence_series(other_evidence_seq) + else: + other_evidence = [] + + evidence = list(chain(current_evidence, other_evidence)) + + # Deduplicate the list while preserving order + evidence = list(dict.fromkeys(evidence)) + + return evidence class EnhancedSR(_SR): diff --git a/tests/test_sr.py b/tests/test_sr.py index e79d4cc1..05ee856e 100644 --- a/tests/test_sr.py +++ b/tests/test_sr.py @@ -3772,6 +3772,20 @@ def test_construction(self): performed_procedure_codes=self._performed_procedures ) assert report.SOPClassUID == '1.2.840.10008.5.1.4.1.1.88.22' + evidence = report.get_evidence() + assert len(evidence) == 1 + assert evidence[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + self._ref_dataset.SOPInstanceUID, + self._ref_dataset.SOPClassUID, + ) + evidence_series = report.get_evidence_series() + assert len(evidence_series) == 1 + assert evidence_series[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + ) def test_construction_content_is_sequence(self): report = EnhancedSR( @@ -3913,6 +3927,21 @@ def test_construction(self): with pytest.raises(AttributeError): assert report.PertinentOtherEvidenceSequence + evidence = report.get_evidence() + assert len(evidence) == 1 + assert evidence[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + self._ref_dataset.SOPInstanceUID, + self._ref_dataset.SOPClassUID, + ) + evidence_series = report.get_evidence_series() + assert len(evidence_series) == 1 + assert evidence_series[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + ) + def test_construction_content_is_sequence(self): report = ComprehensiveSR( evidence=[self._ref_dataset], @@ -4029,6 +4058,96 @@ def test_evidence_multiple_studies(self): with pytest.raises(AttributeError): assert report.PertinentOtherEvidenceSequence + evidence = report.get_evidence() + assert len(evidence) == 2 + assert evidence[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + self._ref_dataset.SOPInstanceUID, + self._ref_dataset.SOPClassUID, + ) + assert evidence[1] == ( + ref_dataset.StudyInstanceUID, + ref_dataset.SeriesInstanceUID, + ref_dataset.SOPInstanceUID, + ref_dataset.SOPClassUID, + ) + evidence_series = report.get_evidence_series() + assert len(evidence_series) == 2 + assert evidence_series[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + ) + assert evidence_series[1] == ( + ref_dataset.StudyInstanceUID, + ref_dataset.SeriesInstanceUID, + ) + + def test_current_and_other_evidence(self): + ref_dataset2 = deepcopy(self._ref_dataset) + ref_dataset2.SeriesInstanceUID = '1.2.3' + ref_dataset2.SOPInstanceUID = '1.2.3' + + report = Comprehensive3DSR( + evidence=[self._ref_dataset, ref_dataset2], + content=self._content, + series_instance_uid=self._series_instance_uid, + series_number=self._series_number, + sop_instance_uid=self._sop_instance_uid, + instance_number=self._instance_number, + institution_name=self._institution_name, + institutional_department_name=self._department_name, + manufacturer=self._manufacturer + ) + ref_evd_items = report.CurrentRequestedProcedureEvidenceSequence + assert len(ref_evd_items) == 1 + unref_evd_items = report.PertinentOtherEvidenceSequence + assert len(unref_evd_items) == 1 + + evidence = report.get_evidence() + assert len(evidence) == 2 + assert evidence[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + self._ref_dataset.SOPInstanceUID, + self._ref_dataset.SOPClassUID, + ) + assert evidence[1] == ( + ref_dataset2.StudyInstanceUID, + ref_dataset2.SeriesInstanceUID, + ref_dataset2.SOPInstanceUID, + ref_dataset2.SOPClassUID, + ) + evidence_series = report.get_evidence_series() + assert len(evidence_series) == 2 + assert evidence_series[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + ) + assert evidence_series[1] == ( + ref_dataset2.StudyInstanceUID, + ref_dataset2.SeriesInstanceUID, + ) + + current_evidence = report.get_evidence( + current_procedure_only=True + ) + assert len(current_evidence) == 1 + assert current_evidence[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + self._ref_dataset.SOPInstanceUID, + self._ref_dataset.SOPClassUID, + ) + current_evidence_series = report.get_evidence_series( + current_procedure_only=True + ) + assert len(current_evidence_series) == 1 + assert current_evidence_series[0] == ( + self._ref_dataset.StudyInstanceUID, + self._ref_dataset.SeriesInstanceUID, + ) + def test_srread(self): report = ComprehensiveSR( evidence=[self._ref_dataset], From eb39c85b73e469b761143f1eceb48b7f8f73e7fa Mon Sep 17 00:00:00 2001 From: Chris Bridge Date: Tue, 24 Sep 2024 10:32:32 -0400 Subject: [PATCH 3/3] Fix to docstrings --- src/highdicom/sr/sop.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/highdicom/sr/sop.py b/src/highdicom/sr/sop.py index b13cd952..f58c5018 100644 --- a/src/highdicom/sr/sop.py +++ b/src/highdicom/sr/sop.py @@ -427,8 +427,7 @@ def get_evidence_series( ------- List[Tuple[highdicom.UID, highdicom.UID]]: List of tuples of UIDs, each representing a single series. Each - tuple consists of (StudyInstanceUID, SeriesInstanceUID, - SOPInstanceUID, SOPClassUID). + tuple consists of (StudyInstanceUID, SeriesInstanceUID). """ def extract_evidence_series(