NiFTi Conversion APIs #223

chinacat567 · 2025-06-30T14:56:56Z

Pull Request Overview

This PR adds support for converting and serving DICOM studies, series, and images as NIfTI files via WADO-RS endpoints.

Adds requests-toolbelt dependency for multipart decoding in the client.
Implements wado_retrieve_nifti utility, new DRF renderers, views, and URL routes for NIfTI retrieval.
Extends the Python client with retrieve_nifti_study and updates acceptance tests.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pyproject.toml	Added `requests-toolbelt` dependency
adit/dicom_web/views.py	Introduced `RetrieveNifti*APIView` classes
adit/dicom_web/utils/wadors_utils.py	Implemented `wado_retrieve_nifti` utility
adit/dicom_web/urls.py	Added URL patterns for NIfTI study/series/image routes
adit/dicom_web/renderers.py	Refactored multipart renderer and added NIfTI renderer
adit-client/adit_client/client.py	Added `retrieve_nifti_study` method in `AditClient`
adit/dicom_web/tests/acceptance/test_wadors.py	Extended acceptance tests for NIfTI study retrieval

Closes #113

adit-client/adit_client/client.py

adit/dicom_web/utils/wadors_utils.py

docker-compose.base.yml

adit/dicom_web/utils/wadors_utils.py

Copilot

Pull Request Overview

This PR adds support for converting and serving DICOM studies, series, and images as NIfTI files via WADO-RS endpoints. It introduces NIfTI conversion capabilities by adding new API views, renderers, and utility functions while extending the Python client with methods to retrieve NIfTI files.

Implements WADO-RS NIfTI endpoints at study, series, and image levels with corresponding multipart renderer
Adds comprehensive NIfTI retrieval utilities with asynchronous processing and temporary file management
Extends the Python client with NIfTI retrieval methods and multipart response handling

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pyproject.toml	Added `requests-toolbelt` dependency for multipart decoding
example.env	Added `DICOM_SAMPLE_DIR` environment variable configuration
docker-compose.base.yml	Added `DICOM_SAMPLE_DIR` environment variable to container
adit/settings/base.py	Added `DICOM_SAMPLE_DIR` setting for sample DICOM files path
adit/dicom_web/views.py	Implemented three new NIfTI retrieval API views for study/series/image levels
adit/dicom_web/utils/wadors_utils.py	Added comprehensive NIfTI conversion utilities and helper classes
adit/dicom_web/urls.py	Added URL patterns for NIfTI endpoints
adit/dicom_web/tests/acceptance/test_wadors.py	Added acceptance tests for NIfTI retrieval functionality
adit/dicom_web/renderers.py	Refactored multipart rendering and added NIfTI-specific renderer
adit-client/adit_client/client.py	Added NIfTI retrieval methods with multipart response handling

adit/dicom_web/utils/wadors_utils.py

adit-client/adit_client/client.py

adit/dicom_web/tests/acceptance/test_wadors.py

adit-client/adit_client/client.py

example.env

adit/dicom_web/utils/wadors_utils.py

adit/settings/base.py

adit/dicom_web/utils/wadors_utils.py

medihack · 2025-07-23T09:39:17Z

adit/dicom_web/utils/wadors_utils.py

+            )
+        except (NoValidDicomError, NoSpatialDataError):
+            # These exceptions are expected for series that cannot be converted to NIfTI
+            # For example, non-image series (e.g., SR documents) or series without spatial data


Do you remember the Claude link I sent you? Please only ignore exceptions for series where we know that they don't contain any images. Otherwise also log a warning.

Are you suggesting that we skip running the conversion on series without images? That seems like a lot of overhead just to avoid attempting conversion. I'm also not sure what the best approach for identifying those series in advance would be.

Personally, I'd rather try converting every series and just catch the ones without image data using the NoValidDicomError and NoSpatialDataError exceptions.

Is there something I'm overlooking here?

No, I suggest not logging anything for series we know that those don't contain images. And log something for other series when they don't contain images (because we would expect that those contain images).

adit/dicom_web/utils/wadors_utils.py

medihack · 2025-07-23T10:50:14Z

adit/dicom_web/utils/wadors_utils.py

+                # For .nii.gz files, strip the .nii part from the base_name
+                actual_base = os.path.splitext(base_name)[0]
+                file_pairs.setdefault(actual_base, {}).update({"nifti": filename})
+            elif ext == ".nii":


What about bval/bvec files? Aditya asked about those in the group meeting (do you remember?). And we should also log a warning if other files are created than we expected.

Extended for bvac/bval files.

The warning (or maybe info?!) for other files is still missing (an additional else block)

I would not expect any other files (maybe perhaps .txt file). But yes, logging this doesn't hurt.

This is precisely what assert is for; to prove your expectation. So assert it then.

adit-client/adit_client/client.py

…rt_message from DICOMWebClient for iter methods with patching

medihack · 2025-07-24T13:55:20Z

adit/dicom_web/utils/wadors_utils.py

+                    yield bval_filename, BytesIO(bval_content)
+
+            # Finally yield the bvec file if it exists (diffusion b-vectors)
+            if "bvec" in files:


The if clauses look very redundant, why not a for loop like this:

for file_type in ["json", "nifti", ...]: filename = files[file_type] ...

medihack · 2025-07-24T15:06:51Z

adit/core/utils/dicom_to_nifti_converter.py


+class DcmExitCode(IntEnum):


Can we rename this DcmToNiftiExitCode? Makes it more clear what exit code it is.

medihack · 2025-07-24T15:09:49Z

adit/settings/test.py

 ]

+# The base directory of the project (the root of the repository)
+BASE_PATH = Path(__file__).resolve(strict=True).parent.parent.parent


The BASE_PATH is already set in base. And as you see in Line 1/3 everything is imported from there.

medihack · 2025-07-24T15:33:09Z

adit/core/errors.py

    pass


+class DicomConversionError(Exception):


I would here also prefer DcmToNiftiConversionError as it's clearer. Also why is this in core? It's only used in dicom_web!

medihack · 2025-07-24T15:34:30Z

adit/core/utils/dicom_to_nifti_converter.py

Why is this in core?! It's part of dicom_web!

medihack · 2025-07-24T15:37:22Z

adit/dicom_web/utils/wadors_utils.py

+        self.query_ds = QueryDataset.from_dict(query)
+        self.operator = DicomOperator(source_server)
+
+    def create_fetch_task(


Why is this still called create_fetch_task? What task does it create? Also, please provide a valid return type as this is a bit strange function. I still wonder if it is not better to keep this method sync and then just use sync_to_async where it is really used. Would also get rid off those redundant sync_to_async calls.

medihack · 2025-07-24T15:56:45Z

adit/dicom_web/utils/wadors_utils.py

+        except Exception as e:
+            # For all other unexpected errors, log and propagate the error
+            logger.error(f"Error during DICOM to NIfTI conversion: {e}")
+            raise


Not necessary for the same reasons as mentioned in this comment: #223 (comment)
Only logging and re-raising doesn't make much sense. Not handled exceptions are logged anyway.

medihack · 2025-07-24T19:59:31Z

pyproject.toml

 line-length = 100
 lint.select = ["E", "F", "I", "DJ"]

+[tool.ruff.lint.isort]


Why that!? I prefer the default order. This PR is not about changing the style guide.

chinacat567 marked this pull request as ready for review July 10, 2025 12:20

chinacat567 requested review from Copilot and medihack July 10, 2025 12:20

This comment was marked as outdated.

Sign in to view

chinacat567 marked this pull request as draft July 10, 2025 16:09

chinacat567 changed the title ~~Nifti conversion api~~ NiFTi Conversion APIs Jul 16, 2025

chinacat567 requested a review from Copilot July 16, 2025 13:00

This comment was marked as outdated.

Sign in to view

chinacat567 force-pushed the nifti_conversion_api branch from c836114 to 608e8d1 Compare July 16, 2025 13:11

chinacat567 marked this pull request as ready for review July 16, 2025 13:35

medihack reviewed Jul 20, 2025

View reviewed changes

adit-client/adit_client/client.py Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

adit-client/adit_client/client.py Outdated Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

docker-compose.base.yml Outdated Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 20, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

chinacat567 marked this pull request as draft July 21, 2025 12:00

chinacat567 requested a review from Copilot July 21, 2025 14:06

Copilot AI reviewed Jul 21, 2025

View reviewed changes

chinacat567 force-pushed the nifti_conversion_api branch from cbb4d5d to 38cfc80 Compare July 21, 2025 14:41

chinacat567 marked this pull request as ready for review July 21, 2025 14:51

medihack reviewed Jul 21, 2025

View reviewed changes

adit-client/adit_client/client.py Outdated Show resolved Hide resolved

medihack reviewed Jul 21, 2025

View reviewed changes

example.env Outdated Show resolved Hide resolved

medihack reviewed Jul 21, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 21, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Show resolved Hide resolved

medihack reviewed Jul 21, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 21, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

chinacat567 marked this pull request as draft July 22, 2025 14:40

chinacat567 marked this pull request as ready for review July 22, 2025 17:12

aiofiles fix

f41d341

chinacat567 force-pushed the nifti_conversion_api branch from 5e57123 to f41d341 Compare July 23, 2025 08:14

medihack reviewed Jul 23, 2025

View reviewed changes

adit/settings/base.py Outdated Show resolved Hide resolved

medihack reviewed Jul 23, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 23, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 23, 2025

View reviewed changes

adit/dicom_web/utils/wadors_utils.py Outdated Show resolved Hide resolved

medihack reviewed Jul 23, 2025

View reviewed changes

adit-client/adit_client/client.py Outdated Show resolved Hide resolved

chinacat567 marked this pull request as draft July 23, 2025 12:39

chinacat567 added 7 commits July 24, 2025 13:29

use list comprehension

0b0b332

adding test settings

7369f43

removing MultipartDecoder for retrieve methods, using _decode_multipa…

c902e49

…rt_message from DICOMWebClient for iter methods with patching

adding tests for iter methods

b925022

fixing await pattern on async function

7edf07c

adding bval/bvac files

af66b1b

dicom to nifti fix

0295bcc

medihack reviewed Jul 24, 2025

View reviewed changes

adit/core/utils/dicom_to_nifti_converter.py

Copy link

Member

medihack Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this in core?! It's part of dicom_web!

medihack reviewed Jul 24, 2025

View reviewed changes

chinacat567 force-pushed the nifti_conversion_api branch 2 times, most recently from bccd322 to 0295bcc Compare July 24, 2025 18:40

chinacat567 added 3 commits July 24, 2025 18:49

testing

ad60d1d

lint fix

967cd6b

restoring ci.yml and pyrpoject

25a4846

medihack reviewed Jul 24, 2025

View reviewed changes


		class DcmExitCode(IntEnum):

NiFTi Conversion APIs #223

Are you sure you want to change the base?

NiFTi Conversion APIs #223

Uh oh!

Conversation

chinacat567 commented Jun 30, 2025 • edited by medihack Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Overview

Reviewed Changes

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

medihack Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

medihack Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

medihack Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

chinacat567 commented Jun 30, 2025 •

edited by medihack

Loading

medihack Jul 24, 2025 •

edited

Loading

medihack Jul 24, 2025 •

edited

Loading

medihack Jul 24, 2025 •

edited

Loading