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-4940][External] - Remove the need to pass the legacy flag when importing and converting NifTI annotations #973

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

JBWilkie
Copy link
Contributor

@JBWilkie JBWilkie commented Nov 22, 2024

Problem

Currently, medical files in the platform may be divided into two groups:

  • 1: Files that were isotropically scaled upon upload - These are all files uploaded with the MED_2D_VEWER FF disabled
  • 2: Files that were not isotropically scaled upon upload - These are all files uploaded with the MED_2D_VIEWER FF enabled

Now that MED_2D_VIEWER is globally enabled, all further uploaded files will not be isotropically scaled

When importing NifTI annotations to these items, or when converting their annotations to the NifTI format, we need to apply exactly the same scaling as was applied to the item itself upon upload. Currently, we control this with the legacy flag, but it's very manual and can be tricky for users to manage

Solution

This PR removes the need to pass the legacy flag in both the import and convert flows by:

  • Importing annotations: Using the API to lookup the state of each file targeted by the import. If it was scaled, we apply scaling to the imported annotations, and vice versa
  • Converting annotations: We use the presence of a newly added Darwin JSON medical metadata field "handler": "MONAI" to determine if a file was scaled or not, and apply scaling during the conversion if required

After this is merged, we'll release a new darwin-py version and update backend with it. This will enable backend to automatically scale exported NifTI annotations per-file, and then we'll fully remove the legacy flag and all related code some weeks in the future

Changelog

Removed the need to pass a flag when importing or converting NifTI annotations. Instead, we handle it automatically, significantly reducing confusion and the likelihood of errors

)

video_annotations = list(annotation_files)
for video_annotation in video_annotations:
try:
medical_metadata = video_annotation.slots[0].metadata
legacy = not medical_metadata.get("handler") == "MONAI" # type: ignore
Copy link
Contributor

@dorfmanrobert dorfmanrobert Nov 25, 2024

Choose a reason for hiding this comment

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

I think we also need to check if is_volume is true

and in is_volume=false cases, we should apply no scaling at all right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notes from our in-person conversation - These are the scenarios:

Before MED_2D_VIEWER:

  • Not MONAI: Files are scaled --> It's possible to upload unscaled files, but these are files uploaded as slices (not volumes), which NifTI annotations cannot be exported from

After MED_2D_VIEWER:

  • MONAI: Files are guaranteed to be unscaled
  • Not MONAI: Files are scaled --> It's possible to upload unscaled files, but these are files uploaded as slices (not volumes), which NifTI annotations cannot be exported from

@JBWilkie JBWilkie merged commit ac0d661 into master Nov 26, 2024
23 checks passed
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.

None yet

2 participants