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

[IO-2040][external] Fix for NifTI exports that include filenames with dots #705

Merged
merged 3 commits into from
Oct 31, 2023

Conversation

JBWilkie
Copy link
Collaborator

@JBWilkie JBWilkie commented Oct 31, 2023

Problem

As described in IO-2040, NifTI exports of files with filenames containing dots would fail due to there being too many suffixes

Solution

Add logic to extract the last 2 suffixes of each filename while preserving the existing error checking logic

Changelog

Fixed a bug preventing NifTI exports that include filenames with dots

@linear
Copy link

linear bot commented Oct 31, 2023

IO-2040 BUG: There is an issue exporting NIfTI that is not an issue for Darwin JSON. Seems to be an issue with having periods in the filename.

BUG submission from: Jade Yip
Summary (describe an issue):
There is an issue exporting NIfTI that is not an issue for Darwin JSON. Seems to be an issue with having periods in the filename.
Has a strategic customer been impacted or reported the issue?
NO
Share Loom/Screenshots with Console/Network opened:
Update: when attempting to export (4) 'Complete' labels on dataset: 683874 via V7 web console, we ran into the following issue:
{"error": "Misconfigured filename, contains too many suffixes"}
https://darwin.v7labs.com/workview?dataset=683874&item=018ab003-b4a8-465a-d0cc-47190b775fee

I'm not sure why periods '.' are not allowed in the V7 NIfTI basename (e.g., __1.0mm__AXIAL.nii.gz)

Possible solution: only parse the last two suffixes from right prior to returning error
if ".nii.gz" in path.name: # since path.stem only returns '.gz'
suffixes = path.suffixes[-2:]

suffixes = filename.suffixes

Is there a way to rename existing V7 resources and/or export the completed NIfTI volumes with python pathlib suffixes > 2 without issue?
Darwin affected version
V2
Environment (production/staging; browser and OS version)
Production
Impact
All (all Darwin users are impacted)
Priority
High - significant number users no straightforward workaround, not blocking
Steps to reproduce
Try to export this file to NIfTI
Open the resulting JSON
See that it has an error
Expected Behaviour
Try to export this file to NIfTI
Export is successful
Team name from Superadmin
PictureHealth
Team & Dataset Link
Team:
https://darwin.v7labs.com/super_admin/teams/3523

Dataset:
https://darwin.v7labs.com/workview?dataset=683874&item=018ab003-b4a8-465a-d0cc-47190b775fee
Intercom ticket
https://app.intercom.com/a/inbox/pb9z3asr/inbox/shared/unassigned/conversation/293929?view=List
ARR

Assigned CSM

@JBWilkie JBWilkie requested a review from Nathanjp91 October 31, 2023 11:45
Copy link
Contributor

@Nathanjp91 Nathanjp91 left a comment

Choose a reason for hiding this comment

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

Possible issue but rest of the code looks good.

darwin/exporter/formats/nifti.py Show resolved Hide resolved
@JBWilkie JBWilkie merged commit c4bead4 into master Oct 31, 2023
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