Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Series filtering out based on description string #351

Merged
merged 34 commits into from
Apr 10, 2024
Merged
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a8aa826
Move placeholder value to central location
jeremyestein Mar 20, 2024
397d658
Return proper HTTP errors inside orthanc raw "/send-to-anon" API call
jeremyestein Mar 21, 2024
ab6371c
Check error code from shell - should use requests API but never mind
jeremyestein Mar 21, 2024
589ee29
Refactor waiting for query into orthanc code
jeremyestein Mar 22, 2024
855f371
Merge branch 'main' into jeremy/series-filtering
jeremyestein Mar 22, 2024
2aea37b
The "correct" test data wasn't being validated to ensure it was actually
jeremyestein Mar 22, 2024
0efb732
Merge branch 'jeremy/config-fixes' into jeremy/series-filtering
jeremyestein Mar 22, 2024
a894f13
Remove unnecessary try except
jeremyestein Mar 25, 2024
1916a3a
Merge branch 'main' into jeremy/series-filtering
jeremyestein Mar 25, 2024
61520d5
Add explanatory comments
jeremyestein Mar 27, 2024
31d34f5
Move private tag creation code to library
jeremyestein Mar 27, 2024
3d1d7ba
Merge branch 'main' into jeremy/series-filtering
jeremyestein Mar 27, 2024
a0afd82
Merge branch 'main' into jeremy/series-filtering
jeremyestein Mar 27, 2024
de4356b
Reuse existing method
jeremyestein Mar 27, 2024
1997d6a
Add series filtering feature and some unit tests (system tests to come)
jeremyestein Mar 27, 2024
958e3b6
Speculative fix for occasional rabbitmq crashes
jeremyestein Mar 27, 2024
be7e094
I think this check got missed
jeremyestein Mar 27, 2024
0ce17ca
First attempt at a system test. The data is not behaving though.
jeremyestein Mar 27, 2024
706c4d4
The fake VNA was not seeing the test instances as part of the same
jeremyestein Mar 28, 2024
a6f5a85
Modality was causing some test instances to get filtered out. Also de…
jeremyestein Mar 28, 2024
684855a
Tidy up system test and move dicom data set up from shell script to
jeremyestein Apr 9, 2024
6702a3d
Merge branch 'main' into jeremy/series-filtering
jeremyestein Apr 9, 2024
4a05bf5
Fix the merge; de-dupe the VR field
jeremyestein Apr 10, 2024
a2a1f51
Some typing fixes needed now that we have mypy enabled
jeremyestein Apr 10, 2024
6625174
Merge branch 'main' into jeremy/series-filtering
jeremyestein Apr 10, 2024
a844cee
Pytest classes cannot have an __init__ method; several tests were being
jeremyestein Apr 10, 2024
6e4b75a
Merge branch 'jeremy/fix-system-test-class' into jeremy/series-filtering
jeremyestein Apr 10, 2024
ad4aaa4
Return accurate content type
jeremyestein Apr 10, 2024
493fd66
Make series description test case-insensitive
jeremyestein Apr 10, 2024
21f7f1b
Remove defunct comment
jeremyestein Apr 10, 2024
7e9bd41
Tidy up logging
jeremyestein Apr 10, 2024
0eec011
Merge branch 'main' into jeremy/series-filtering
jeremyestein Apr 10, 2024
debf08c
Get the media type actually right this time.
jeremyestein Apr 10, 2024
2d71237
Move method to utility module
jeremyestein Apr 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'main' into jeremy/series-filtering
jeremyestein committed Apr 9, 2024
commit 6702a3d87d73c4c5d7aedbb5cb1a30c5b0418c8e
7 changes: 3 additions & 4 deletions orthanc/orthanc-raw/plugin/pixl.py
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@
from io import BytesIO
from typing import TYPE_CHECKING, Optional

from core.dicom_tags import DICOM_TAG_PROJECT_NAME
from core.dicom_tags import DICOM_TAG_PROJECT_NAME, add_private_tag
from pydicom import dcmread

import orthanc
@@ -111,9 +111,8 @@ def modify_dicom_tags(receivedDicom: bytes, origin: str) -> Any:
# hardcoded to 0x10
# https://dicom.nema.org/dicom/2013/output/chtml/part05/sect_7.8.html

private_block = DICOM_TAG_PROJECT_NAME.add_to_dicom_dataset(
dataset, DICOM_TAG_PROJECT_NAME.PLACEHOLDER_VALUE
)
# Add project name as private tag, at this point, the value is unknown
private_block = add_private_tag(dataset, DICOM_TAG_PROJECT_NAME)

logger.debug(
"modify_dicom_tags - added new private block starting at 0x%x", private_block.block_start
39 changes: 20 additions & 19 deletions pixl_core/src/core/dicom_tags.py
Original file line number Diff line number Diff line change
@@ -24,10 +24,12 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import Any
from typing import TYPE_CHECKING, Any, Optional

from pydicom import Dataset
from pydicom.dataset import PrivateBlock

if TYPE_CHECKING:
from pydicom.dataset import Dataset, PrivateBlock


@dataclass
@@ -67,19 +69,6 @@ def acceptable_private_block(self, actual_private_block: int) -> bool:
return True
return self.required_private_block == actual_private_block

def add_to_dicom_dataset(self, dataset: Dataset, value: Any) -> PrivateBlock:
"""Add this private tag to the given dicom dataset, setting the given value"""
private_block = dataset.private_block(self.group_id, self.creator_string, create=True)
private_block.add_new(self.offset_id, self.value_type, value)
if not self.acceptable_private_block(private_block.block_start >> 8):
err_str = (
"The private block does not match the value hardcoded in the orthanc "
"config. This can be because there was an unexpected pre-existing private block "
f"in group {self.group_id}"
)
raise RuntimeError(err_str)
return private_block


DICOM_TAG_PROJECT_NAME = PrivateDicomTag(
group_id=0x000D,
@@ -93,21 +82,33 @@ def add_to_dicom_dataset(self, dataset: Dataset, value: Any) -> PrivateBlock:
)


def add_private_tag(dataset: Dataset, private_tag: PrivateDicomTag) -> PrivateBlock:
def add_private_tag(
dataset: Dataset, private_tag: PrivateDicomTag, value: Optional[str] = None
) -> PrivateBlock:
"""
Add a private tag to an existing DICOM dataset.

This uses pydicom.Dataset.private_block

:param ds: The DICOM dataset to add the private tags to.
:type ds: pydicom.Dataset
:param dataset: The DICOM dataset to add the private tags to.
:type dataset: pydicom.Dataset
:param private_tag: A custom tag to add to the DICOM dataset.
:param value: Optional value string. If None, use the default placeholder value.

"""
private_block = dataset.private_block(
private_tag.group_id, private_tag.creator_string, create=True
)
private_block.add_new(private_tag.offset_id, private_tag.vr, private_tag.unknown_value)
if value is None:
value = private_tag.unknown_value
private_block.add_new(private_tag.offset_id, private_tag.vr, value)
if not private_tag.acceptable_private_block(private_block.block_start >> 8):
err_str = (
"The private block does not match the value hardcoded in the orthanc "
"config. This can be because there was an unexpected pre-existing private block "
f"in group {private_tag.group_id}"
)
raise RuntimeError(err_str)
return private_block


3 changes: 1 addition & 2 deletions pixl_dcmd/tests/test_main.py
Original file line number Diff line number Diff line change
@@ -34,7 +34,6 @@

from pydicom.dataset import Dataset

from core.dicom_tags import DICOM_TAG_PROJECT_NAME
from pytest_pixl.dicom import generate_dicom_dataset
from pytest_pixl.helpers import run_subprocess

@@ -237,7 +236,7 @@ def dicom_series_to_exclude() -> list[Dataset]:

def _make_dicom(series_description) -> Dataset:
ds = generate_dicom_dataset(SeriesDescription=series_description)
DICOM_TAG_PROJECT_NAME.add_to_dicom_dataset(ds, "test-extract-uclh-omop-cdm")
add_private_tag(ds, DICOM_TAG_PROJECT_NAME, "test-extract-uclh-omop-cdm")
return ds


5 changes: 3 additions & 2 deletions test/conftest.py
Original file line number Diff line number Diff line change
@@ -14,13 +14,14 @@
"""System/E2E test setup"""

# ruff: noqa: C408 dict() makes test data easier to read and write

from collections.abc import Generator
from pathlib import Path
from typing import Any

import pytest
import requests
from pytest_pixl.dicom import generate_dicom_dataset
from pytest_pixl.ftpserver import PixlFTPServer
from pytest_pixl.helpers import run_subprocess
from pytest_pixl.plugin import FtpHostAddress
from utils import wait_for_stable_orthanc_anon
@@ -133,7 +134,7 @@ def _upload_dicom_instance(dicom_dir: Path, **kwargs) -> None:


@pytest.fixture(scope="session")
def _setup_pixl_cli(ftps_server, _populate_vna) -> None:
def _setup_pixl_cli(ftps_server: PixlFTPServer, _populate_vna) -> Generator:
"""Run pixl populate/start. Cleanup intermediate export dir on exit."""
# CLI calls need to have CWD = test dir so they can find the pixl_config.yml file
run_subprocess(["pixl", "populate", str(RESOURCES_OMOP_DIR.absolute())], TEST_DIR)
4 changes: 3 additions & 1 deletion test/system_test.py
Original file line number Diff line number Diff line change
@@ -108,7 +108,9 @@ def two_zip_files_present() -> bool:
unzip_dir = tmp_path_factory.mktemp("unzip_dir", numbered=True)
self._check_dcm_tags_from_zip(z, unzip_dir, expected_studies)

def _check_dcm_tags_from_zip(self, zip_path, unzip_dir, expected_studies) -> None:
def _check_dcm_tags_from_zip(
self, zip_path: str, unzip_dir: Path, expected_studies: dict[str, set[str]]
) -> None:
"""Check that private tag has survived anonymisation with the correct value."""
expected_instances = expected_studies[zip_path.stem]
run_subprocess(
2 changes: 1 addition & 1 deletion test/utils.py
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@


def wait_for_stable_orthanc_anon(
seconds_max, seconds_interval, seconds_condition_stays_true_for
seconds_max: int, seconds_interval: int, seconds_condition_stays_true_for: int
) -> None:
"""
Query the orthanc-anon REST API to check that the correct number of instances
You are viewing a condensed version of this merge commit. You can view the full changes here.