From e5376c2ea2966ffa54804cbb9bdcc6a6042e0a79 Mon Sep 17 00:00:00 2001 From: Stefan Piatek Date: Mon, 3 Aug 2026 09:47:46 +0100 Subject: [PATCH 01/12] Encapsulate compressed pixel data --- pixl_dcmd/src/pixl_dcmd/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pixl_dcmd/src/pixl_dcmd/main.py b/pixl_dcmd/src/pixl_dcmd/main.py index 95b73c92..fb6869b5 100644 --- a/pixl_dcmd/src/pixl_dcmd/main.py +++ b/pixl_dcmd/src/pixl_dcmd/main.py @@ -262,7 +262,7 @@ def _clean_dicom_image_pixels( burned_pixels = has_burned_pixels(dataset, deid=deid_recipe) cleaned_pixels = clean_pixel_data(dicom_file=dataset, results=burned_pixels) - dataset.PixelData = cleaned_pixels.tobytes() + dataset.PixelData = pydicom.encaps.encapsulate([cleaned_pixels.tobytes()]) def _anonymise_dicom_from_scheme( From 33e58f80bd5e1909fc55e74c0580dfe10e781cd0 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 15:20:44 +0100 Subject: [PATCH 02/12] Tmp change to explicit FTPS --- pixl_core/src/core/uploader/_ftps.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 1fa91b7e..736030cc 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -38,6 +38,7 @@ class ImplicitFtpTls(ftplib.FTP_TLS): """ FTP_TLS subclass that automatically wraps sockets in SSL to support implicit FTPS. + Use explicit TLS where possible. https://stackoverflow.com/questions/12164470/python-ftp-implicit-tls-connection-issue """ @@ -171,7 +172,7 @@ def upload_parquet_files(self, parquet_export: ParquetExport) -> None: def _connect_to_ftp(ftp_host: str, ftp_port: int, ftp_user: str, ftp_password: str) -> FTP_TLS: # Connect to the server and login try: - ftp = ImplicitFtpTls() + ftp = ftplib.FTP_TLS() ftp.connect(ftp_host, int(ftp_port)) ftp.login(ftp_user, ftp_password) ftp.prot_p() From 114eaf353745f55f78607a9e93092d4689e9ca59 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:02:25 +0100 Subject: [PATCH 03/12] Add FTPES as a destination --- README.md | 4 +++- .../core/project_config/pixl_config_model.py | 1 + pixl_core/src/core/uploader/__init__.py | 3 ++- pixl_core/src/core/uploader/_ftps.py | 22 +++++++++++++++---- ...ng-for-timely-rapid-assessment-of-cts.yaml | 4 ++-- 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 73c00b6a..d8167aca 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,9 @@ The configuration file defines: - The endpoints used to upload the anonymised DICOM data and the public and radiology [parquet files](./docs/file_types/parquet_files.md). We currently support the following endpoints: - `"none"`: no upload - - `"ftps"`: a secure FTP server (for both _DICOM_ and _parquet_ files) + - `"ftps"`: a secure FTP server using implicit TLS (for both _DICOM_ and _parquet_ files), e.g. UCL DSH + - `"ftpes"`: a secure FTP server using explicit TLS (for both _DICOM_ and _parquet_ files) + - `"dicomweb"`: a DICOMweb server (for _DICOM_ files only). Requires the `DICOMWEB_*` environment variables to be set in `.env` - `"xnat"`: an [XNAT](https://www.xnat.org/) instance (for _DICOM_ files only) diff --git a/pixl_core/src/core/project_config/pixl_config_model.py b/pixl_core/src/core/project_config/pixl_config_model.py index f52c3584..c4b31d77 100644 --- a/pixl_core/src/core/project_config/pixl_config_model.py +++ b/pixl_core/src/core/project_config/pixl_config_model.py @@ -146,6 +146,7 @@ class _DestinationEnum(enum.StrEnum): none = "none" ftps = "ftps" + ftpes = "ftpes" dicomweb = "dicomweb" xnat = "xnat" tre = "tre" diff --git a/pixl_core/src/core/uploader/__init__.py b/pixl_core/src/core/uploader/__init__.py index b244ebdc..b5fc58ad 100644 --- a/pixl_core/src/core/uploader/__init__.py +++ b/pixl_core/src/core/uploader/__init__.py @@ -28,7 +28,7 @@ from core.project_config import load_project_config from ._dicomweb import DicomWebUploader -from ._ftps import FTPSUploader +from ._ftps import FTPESUploader, FTPSUploader from ._treapi import TreApiUploader from ._xnat import XNATUploader @@ -41,6 +41,7 @@ def get_uploader(project_slug: str) -> Uploader: """Uploader Factory, returns uploader instance based on destination.""" choices: dict[str, type[Uploader]] = { "ftps": FTPSUploader, + "ftpes": FTPESUploader, "dicomweb": DicomWebUploader, "xnat": XNATUploader, "tre": TreApiUploader, diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 736030cc..652772db 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -64,6 +64,8 @@ def sock(self, value: socket | None) -> None: class FTPSUploader(Uploader): """Upload strategy for an FTPS server.""" + ftp_tls_class: type[ftplib.FTP_TLS] = ImplicitFtpTls + def __init__(self, project_slug: str, keyvault_alias: str | None) -> None: """Create instance of parent class""" super().__init__(project_slug, keyvault_alias) @@ -96,7 +98,7 @@ def send_via_ftps( ) -> None: """Send the zip content to the FTPS server.""" # Create the remote directory if it doesn't exist - ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) + ftp = _connect_to_ftp(self.host, self.port, self.user, self.password, self.ftp_tls_class) _create_and_set_as_cwd(ftp, remote_directory) command = f"STOR {pseudo_anon_image_id}.zip" logger.debug("Running {}", command) @@ -134,7 +136,7 @@ def upload_parquet_files(self, parquet_export: ParquetExport) -> None: source_root_dir = parquet_export.current_extract_base # Create the remote directory if it doesn't exist - ftp = _connect_to_ftp(self.host, self.port, self.user, self.password) + ftp = _connect_to_ftp(self.host, self.port, self.user, self.password, self.ftp_tls_class) _create_and_set_as_cwd(ftp, parquet_export.project_slug) _create_and_set_as_cwd(ftp, parquet_export.extract_time_slug) _create_and_set_as_cwd(ftp, "parquet") @@ -169,10 +171,22 @@ def upload_parquet_files(self, parquet_export: ParquetExport) -> None: logger.info("Finished FTPS upload of files for '{}'", parquet_export.project_slug) -def _connect_to_ftp(ftp_host: str, ftp_port: int, ftp_user: str, ftp_password: str) -> FTP_TLS: +class FTPESUploader(FTPSUploader): + """Upload strategy for an FTPES server (explicit rather than implicit TLS).""" + + ftp_tls_class = ftplib.FTP_TLS + + +def _connect_to_ftp( + ftp_host: str, + ftp_port: int, + ftp_user: str, + ftp_password: str, + ftp_tls_class: type[ftplib.FTP_TLS], +) -> FTP_TLS: # Connect to the server and login try: - ftp = ftplib.FTP_TLS() + ftp = ftp_tls_class() ftp.connect(ftp_host, int(ftp_port)) ftp.login(ftp_user, ftp_password) ftp.prot_p() diff --git a/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml b/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml index 883b52c2..d43ca880 100644 --- a/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml +++ b/projects/configs/ultrasound-learning-for-timely-rapid-assessment-of-cts.yaml @@ -39,5 +39,5 @@ series_filters: - "positioning" destination: - dicom: "ftps" - parquet: "ftps" + dicom: "ftpes" + parquet: "ftpes" From c287bd8b278a83e8a305fa4ff37f948b20911152 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:08:57 +0100 Subject: [PATCH 04/12] Tmp don't change directory --- pixl_core/src/core/uploader/_ftps.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 652772db..16f60dd7 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -209,12 +209,4 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N _create_and_set_as_cwd(ftp, sd) -def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: - try: - ftp.mkd(project_dir) - except ftplib.error_perm: - logger.debug("'{}' exists on remote ftp, so moving into it", project_dir) - else: - logger.info("created '{}' on remote ftp and moving into it", project_dir) - - ftp.cwd(project_dir) +def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: ... From b17cb43ac0232a86ac736ae8b65833b8ab9c2080 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:15:29 +0100 Subject: [PATCH 05/12] Log current dir --- pixl_core/src/core/uploader/_ftps.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 16f60dd7..46b4823a 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -209,4 +209,5 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N _create_and_set_as_cwd(ftp, sd) -def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: ... +def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: + logger.warning("cwd: {}", ftp.pwd()) From 9d63ccb51451500a9032c5beca52b98c931c5d69 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:19:10 +0100 Subject: [PATCH 06/12] Revert "Log current dir" This reverts commit b17cb43ac0232a86ac736ae8b65833b8ab9c2080. --- pixl_core/src/core/uploader/_ftps.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 46b4823a..16f60dd7 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -209,5 +209,4 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N _create_and_set_as_cwd(ftp, sd) -def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: - logger.warning("cwd: {}", ftp.pwd()) +def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: ... From b60d4acf543309c72555d843afa3125722c6632e Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:19:24 +0100 Subject: [PATCH 07/12] Revert "Tmp don't change directory" This reverts commit c287bd8b278a83e8a305fa4ff37f948b20911152. --- pixl_core/src/core/uploader/_ftps.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 16f60dd7..652772db 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -209,4 +209,12 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N _create_and_set_as_cwd(ftp, sd) -def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: ... +def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: + try: + ftp.mkd(project_dir) + except ftplib.error_perm: + logger.debug("'{}' exists on remote ftp, so moving into it", project_dir) + else: + logger.info("created '{}' on remote ftp and moving into it", project_dir) + + ftp.cwd(project_dir) From 4aa8e419258e4ac47d40a59871c3cebafca2688d Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:23:37 +0100 Subject: [PATCH 08/12] tmp dir contents --- pixl_core/src/core/uploader/_ftps.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 652772db..e67e408a 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -210,6 +210,8 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: + logger.warning("cwd: {}", ftp.pwd()) + logger.warning("contents: {}", ftp.list()) try: ftp.mkd(project_dir) except ftplib.error_perm: From f3d1ab15fac3cf25ef483fcae478954c3566e558 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 16:26:23 +0100 Subject: [PATCH 09/12] tmp dir contents --- pixl_core/src/core/uploader/_ftps.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index e67e408a..42bf889f 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -211,7 +211,9 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: logger.warning("cwd: {}", ftp.pwd()) - logger.warning("contents: {}", ftp.list()) + logger.warning("contents: {}", ftp.retrlines('LIST')) + ftp.cwd("ftp") + try: ftp.mkd(project_dir) except ftplib.error_perm: From 8b2fa783cb3ccd965dc3b631d0eff6f87c51da1a Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 17:03:12 +0100 Subject: [PATCH 10/12] Remove tmp lines --- pixl_core/src/core/uploader/_ftps.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pixl_core/src/core/uploader/_ftps.py b/pixl_core/src/core/uploader/_ftps.py index 42bf889f..652772db 100644 --- a/pixl_core/src/core/uploader/_ftps.py +++ b/pixl_core/src/core/uploader/_ftps.py @@ -210,10 +210,6 @@ def _create_and_set_as_cwd_multi_path(ftp: FTP_TLS, remote_multi_dir: Path) -> N def _create_and_set_as_cwd(ftp: FTP_TLS, project_dir: str) -> None: - logger.warning("cwd: {}", ftp.pwd()) - logger.warning("contents: {}", ftp.retrlines('LIST')) - ftp.cwd("ftp") - try: ftp.mkd(project_dir) except ftplib.error_perm: From 81cd7ec9b02cbc862021bdb71328cf98a43caa79 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Mon, 3 Aug 2026 17:03:20 +0100 Subject: [PATCH 11/12] Keep in requires US tags --- projects/configs/tag-operations/us.yaml | 46 ++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/projects/configs/tag-operations/us.yaml b/projects/configs/tag-operations/us.yaml index f755dfbc..77a66c89 100644 --- a/projects/configs/tag-operations/us.yaml +++ b/projects/configs/tag-operations/us.yaml @@ -23,4 +23,48 @@ - name: Sequence of Ultrasound Regions group: 0x0018 element: 0x6011 - op: keep \ No newline at end of file + op: keep +- name: Region Spatial Format + group: 0x0018 + element: 0x6012 + op: keep +- name: Region Data Type + group: 0x0018 + element: 0x6014 + op: keep +- name: Region Flags + group: 0x0018 + element: 0x6016 + op: keep +- name: Region Location Min X0 + group: 0x0018 + element: 0x6018 + op: keep +- name: Region Location Min Y0 + group: 0x0018 + element: 0x601A + op: keep +- name: Region Location Max X1 + group: 0x0018 + element: 0x601C + op: keep +- name: Region Location Max Y1 + group: 0x0018 + element: 0x601E + op: keep +- name: Physical Units X Direction + group: 0x0018 + element: 0x6024 + op: keep +- name: Physical Units Y Direction + group: 0x0018 + element: 0x6026 + op: keep +- name: Physical Delta X + group: 0x0018 + element: 0x602C + op: keep +- name: Physical Delta Y + group: 0x0018 + element: 0x602E + op: keep From 94053cb011f9e17887744adaa13e9081594fa2b3 Mon Sep 17 00:00:00 2001 From: Stef Piatek Date: Tue, 4 Aug 2026 11:56:24 +0100 Subject: [PATCH 12/12] Conditionally encapsulate cleaned pixels --- pixl_dcmd/src/pixl_dcmd/main.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pixl_dcmd/src/pixl_dcmd/main.py b/pixl_dcmd/src/pixl_dcmd/main.py index fb6869b5..8fbbfad3 100644 --- a/pixl_dcmd/src/pixl_dcmd/main.py +++ b/pixl_dcmd/src/pixl_dcmd/main.py @@ -262,7 +262,11 @@ def _clean_dicom_image_pixels( burned_pixels = has_burned_pixels(dataset, deid=deid_recipe) cleaned_pixels = clean_pixel_data(dicom_file=dataset, results=burned_pixels) - dataset.PixelData = pydicom.encaps.encapsulate([cleaned_pixels.tobytes()]) + pixel_bytes = cleaned_pixels.tobytes() + if dataset.file_meta.TransferSyntaxUID.is_compressed: + dataset.PixelData = pydicom.encaps.encapsulate([pixel_bytes]) + else: + dataset.PixelData = pixel_bytes def _anonymise_dicom_from_scheme(