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

[DAR-6238][External] Allow to import annotations without pixdims, affine of plane_map #1039

Merged
merged 4 commits into from
Mar 26, 2025

Conversation

umbertoDifa
Copy link
Collaborator

Problem

Some DICOM files might not have a defined pixdim value. That is the case for ultrasound images.

Solution

Gracefully handle those scenarios by printing a warning and skipping pixdim calculations.

Changelog

Fix bug blocking import for DICOMs without pixdims

@Copilot Copilot bot review requested due to automatic review settings March 25, 2025 15:26
Copy link

linear bot commented Mar 25, 2025

@umbertoDifa umbertoDifa requested review from saurbhc and dorfmanrobert and removed request for Copilot March 25, 2025 15:26
Copy link

@DawidMyslak DawidMyslak left a comment

Choose a reason for hiding this comment

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

I'm reviewing the backend PR for the very first time (don't trust me), I see we're trying to build protection against missing pixdims or the plane map.

My only worry is this:
https://github.com/v7labs/darwin-frontend/blob/master/src/modules/Editor/MedicalMetadata.ts#L67

So if the metadata.medical exists on the slot, both pixdims and plane_map must be present with expected typing. We're creating a risk of errors like "Cannot read properties of undefined".

I think it would be good to default pixdims to be [1, 1, 1] and plane map to:

       plane_map = {
          [slot_name]: metadata.primary_plane || 'AXIAL',
        }

@umbertoDifa umbertoDifa merged commit 7d2aa78 into master Mar 26, 2025
20 checks passed
@umbertoDifa umbertoDifa deleted the dar-6238 branch March 26, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants