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

[ENH] Introduce GIFTI formats in derivatives #1333

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

effigies
Copy link
Collaborator

Closes #1317.

@effigies effigies added this to the 1.8.1 milestone Oct 24, 2022
@codecov
Copy link

codecov bot commented Oct 24, 2022

Codecov Report

Base: 88.39% // Head: 88.39% // No change to project coverage 👍

Coverage data is based on head (154e938) compared to base (49e3c99).
Patch has no changes to coverable lines.

❗ Current head 154e938 differs from pull request most recent head d007fb1. Consider uploading reports for the commit d007fb1 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1333   +/-   ##
=======================================
  Coverage   88.39%   88.39%           
=======================================
  Files          11       11           
  Lines        1086     1086           
=======================================
  Hits          960      960           
  Misses        126      126           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@effigies effigies force-pushed the enh/cifti-and-gifti branch 3 times, most recently from 7dfc96a to 1abca69 Compare November 3, 2022 20:55
@sappelhoff
Copy link
Member

@effigies do you believe we have enough community buy-in for this? Or should we advertise this idea more broadly to collect more opinions?

@effigies
Copy link
Collaborator Author

@sappelhoff Thanks for moving this to 1.9.0 and prompting me to revisit.

I would at least like @neurolabusc's input here, particularly because GIFTI as specified and GIFTI as supported are two different things. I do not want to imply that BIDS endorses a to-spec reading of GIFTI, but nor do I think it is the place to attempt to explain GIFTI-as-supported and confine BIDS to that.

My goal here is to say "where appropriate, we will permit CIFTI-2 and GIFTI file structures, and here's where to go for more information when you see these".

@neurolabusc
Copy link

@effigies in my mind, CIFTI and GIFTI are two separate issues.

GIFTI over the last couple month I have spent a bit of time with GIfTI and evaluating different tools. This has become one of the popular interchange standards in our field. Any format has its tradeoffs. GIFTI's limitations are speed, size and the fact that extensibility means that it can be interpreted and used differently. The advantages are that it is human readable, widely adopted and the extensibility means it can be adapted to fill emerging and tool specific niches.

Many tools implement GIfTI a little differently, and in general they ignore triangle winding. We have recently identified issues with FSL, FSL, nibabel, FreeSurfer, FreeSurfer, FreeSurfer, NiiVue. However, these issues are fixable (and in many of the identified cases already fixed) and the needs to harmonize implementations should be expected with any format. In one sense, this variability highlights the pervasive popularity of GIFTI in our field. It filled a Darwinian niche and is widely adopted.

CIFTI-2 From my perspective, this format was created by the connectome group to fill their specific use case. It is not yet widely adopted in our field. This format was not created by a consensus building across the community and is not yet widespread. I do think there is a use case and a niche for a format like CIFTI, but I do think there should be a community discussion of whether CIFTI-2 should be adopted or another more BIDS-like format that can adopt a similar role. Selecting a community format has implications, and we need to consider how this is implemented across languages, performance, extended and the level of human readability (e.g. JSON vs XML). The fact that CIFTI mixes NIfTI binary headers and an XML extension means it feels unnatural to both those who like the speed/size/explicitness of binary as well as those who like the readability and extensibility of text information. One wonders if a NRRD-like solution or BSON solution might be more harmonious. For example, work by @fangq with neurojson hints at formats that can have simplicity, readability, extensibility, speed and file size. I am not saying we should not adopt CIFTI as a community, but I do think we should carefully consider this choice, evaluate alternatives, and develop a consensus.

@effigies
Copy link
Collaborator Author

Thanks for the perspective, Chris! If I'm reading you correctly, we should split out the GIFTI and CIFTI-2 proposals separately, and raise the CIFTI-2 one for community discussion of general coordinate image data.

Are you okay with the GIFTI section as written?

@neurolabusc
Copy link

@effigies that is my opinion: GIFTI is already widely supported with libraries in many languages and broad tool support.

@effigies effigies changed the title ENH: Introduce GIFTI and CIFTI-2 formats in derivatives ENH: Introduce GIFTI formats in derivatives Mar 16, 2023
@effigies
Copy link
Collaborator Author

CIFTI-2 dropped from this PR. Will open a separate discussion.

Copy link
Collaborator

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

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

OK with me
the CI failure is unrelated

@effigies effigies merged commit e193eed into bids-standard:master Jun 2, 2023
@sappelhoff sappelhoff changed the title ENH: Introduce GIFTI formats in derivatives [ENH] Introduce GIFTI formats in derivatives Nov 13, 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.

Add new derivative file extensions
4 participants