Skip to content

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Apr 4, 2025

The BIDS convention is to use dir-{AP,PA,LR,RL} for phase-encoding directions in MRI filenames, as well as PhaseEncodingDirection in JSON sidecars. This duplication has the potential for inconsistency or multiple interpretations of the meaning of AP.

This PR aims to clarify the meaning of dir-<label> when its value matches a cardinal axis, and clearly state that this meaning SHOULD be consistent with the PhaseEncodingDirection.

Copy link
Collaborator

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

This is great to make the situation better without introducing backward incompatible changes. I made some suggestions to be more explicit about what the problem is and providing examples, that said, the phrasing will likely require improvements.

@effigies effigies force-pushed the feat/nifti-axis-codes branch from d9d762b to a5bad79 Compare May 22, 2025 19:13
Copy link

codecov bot commented May 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.71%. Comparing base (04faf74) to head (aacae58).
⚠️ Report is 77 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2097   +/-   ##
=======================================
  Coverage   82.71%   82.71%           
=======================================
  Files          20       20           
  Lines        1608     1608           
=======================================
  Hits         1330     1330           
  Misses        278      278           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@effigies
Copy link
Collaborator Author

effigies commented Jun 2, 2025

@oesteban Would you mind giving this another review?

Copy link
Collaborator

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Looks good. A couple of suggestions to make things more explicit, but I think this is really useful.

@effigies effigies force-pushed the feat/nifti-axis-codes branch from 570267d to 523402a Compare August 4, 2025 19:28
@effigies effigies added this to the 1.10.1 milestone Aug 4, 2025
@effigies effigies merged commit 9854498 into bids-standard:master Aug 22, 2025
27 checks passed
@effigies effigies deleted the feat/nifti-axis-codes branch August 22, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants