search

freesurfer / surfa

Python utilities for medical image and surface-based analysis
24 stars 8 forks source link

add Freesurfer Nifti1 header extension handling in NiftiArrayIO #27

Closed yhuang43 closed 5 months ago

yhuang43 commented 5 months ago

If the extension data has zeros at the end, line 629 in nibabel/nifti1.py: 'evalue = evalue.rstrip(b'\x00')' will truncate the data when calling Nifti1Extension.get_content() - https://github.com/nipy/nibabel/blob/master/nibabel/nifti1.py#L629C1-L630C1

I will open an issue with Nibabel for this.

I think Doug in the meeting suggested adding extra characters at the end. So, we don't have to rely on Nibabel fix.

yhuang43 commented 5 months ago

Thanks Jackson! In load(), tr is converted to ms; in save(), it is converted to s for nifti header.

I have one more commit for this PR.

jnolan14 commented 5 months ago

Ahhh, ok that makes sense, thanks!

Looking at the second commit now

From: yhuang43 @.> Sent: Friday, March 29, 2024 1:42 PM To: freesurfer/surfa @.> Cc: Nolan, Jackson @.>; Review requested @.> Subject: Re: [freesurfer/surfa] add Freesurfer Nifti1 header extension handling in NiftiArrayIO (PR #27)

    External Email - Use Caution

Thanks Jackson! In load(), tr is converted to ms; in save(), it is converted to s for nifti header.

I have one more commit for this PR.

— Reply to this email directly, view it on GitHubhttps://secure-web.cisco.com/1p0Og2KDUroxo8UgOAx01DIwFS-_6BE9DPILQFbtkcUzuwBbCO6jKwQdeuxLdgC0zRDQhzk3IeaNUz1ywEiChSQeRKPeYkA6EjgGdQQ3J6_nnnBDxUyzAZGsMK54OpF8bSH2qlx4wD3dq3dsu9MG0AZ6VZ3f-0pAV2H0n-EzKluiZQgE_GH2BqDJVfWMGeR4qNPRsEWVFVvgQ2Xsb6jWO6Ak9z0_Ya4RPi96AqUpJokeE-YZtEgzQSBeYhIdsBmdSHTsgJbV2W3X3KaZYHiCX7TDMIPu_YpOAysJogWsVtN5PbDKJ3C9fzSqJ2cm3fema/https%3A%2F%2Fgithub.com%2Ffreesurfer%2Fsurfa%2Fpull%2F27%23issuecomment-2027537401, or unsubscribehttps://secure-web.cisco.com/1AqoJbLxT0biTgdmgGESmq5d33nRL8rP3mEF1NlvZThPFjaiqVvarL_uTXApwGvHdIZjsTVnBWe_z9PQSZnl9jyAj4U33L4uHU8FBjLZoVjge6EuDtEfPgGXfyOnLeIeVShd2FOuAXb-oKOgPgZLI_EaJRQvSjC88nhHEgHLwj3qqo-8D5XVbAa8P6ijGQCyC4vEG8zyxbMMwT-_vb42bY3PMeyXjV8TMdjAe8CE1U2dcoRekOTZU86_tEgSecEjyGJcwh-E7ogKcoaqLjsb10L4Lbqt5yrXbeaMXt8Xh3_H3mB1hqOk0x2srDh-xnTzw/https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FASNYUGRRR5SXGE66TGYHXHTY2WRZVAVCNFSM6AAAAABFJOLE6CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXGUZTONBQGE. You are receiving this because your review was requested.Message ID: @.***> The information in this e-mail is intended only for the person to whom it is addressed. If you believe this e-mail was sent to you in error and the e-mail contains patient information, please contact the Mass General Brigham Compliance HelpLine at https://www.massgeneralbrigham.org/complianceline https://www.massgeneralbrigham.org/complianceline . Please note that this e-mail is not secure (encrypted). If you do not wish to continue communication over unencrypted e-mail, please notify the sender of this message immediately. Continuing to send or respond to e-mail after receiving this message means you understand and accept this risk and wish to continue to communicate over unencrypted e-mail.

yhuang43 commented 5 months ago

Your understanding is correct. The fname field for geom has different formats on disk for nifti header extension and mgz/mgh. for mgz/mgh, it is padded 512 bytes; for nifti header extension, it contains two parts: fname-length, fname. So, for nifti header extension read, it knows exactly how many bytes to expect for fname.

read_geom()/write_geom() are utility functions not associated with any class. Their callers are already determined by the *_io_protocols class. I'm also trying to make io.fsnifti1extension have less dependency so that we can fit it into Nibabel somehow.

The fix to the geom fname doesn't fully resolve nibabel truncation problems. There will still be problems if the last values in our extension data are zeros. I'm introducing a TAG_END_DATA to make sure that won't happen. One more commit is coming.