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

Change T1wACPC to ACPC and T1wNative to anat #859

Merged
merged 12 commits into from
Nov 13, 2024
Merged

Change T1wACPC to ACPC and T1wNative to anat #859

merged 12 commits into from
Nov 13, 2024

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Nov 11, 2024

Closes #853 and closes #854.

Changes proposed in this pull request

  • Rename T1wACPC space value to ACPC.
    • In several cases, T1w is used where ACPC is meant.
    • In some cases, no space entity was used where ACPC was more appropriate.
  • Rename T1wNative space value to anat.

@tsalo tsalo added this to the HBCD First Release milestone Nov 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.57%. Comparing base (0e21c37) to head (cb44ca5).
Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #859   +/-   ##
=======================================
  Coverage   28.57%   28.57%           
=======================================
  Files          68       68           
  Lines       10126    10126           
  Branches     1135     1135           
=======================================
  Hits         2893     2893           
  Misses       7134     7134           
  Partials       99       99           

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

@tsalo tsalo marked this pull request as ready for review November 11, 2024 16:17
@tsalo tsalo requested a review from mattcieslak November 11, 2024 16:18
@tsalo tsalo added the breaking-change Issues/PRs that change results or interfaces. label Nov 11, 2024
Copy link
Collaborator

@mattcieslak mattcieslak left a comment

Choose a reason for hiding this comment

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

Only a few questions

.circleci/DSCSDSI_outputs.txt Show resolved Hide resolved
.circleci/DSDTI_outputs.txt Show resolved Hide resolved
docs/preprocessing.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@mattcieslak mattcieslak 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 to go!

qsiprep/workflows/dwi/finalize.py Outdated Show resolved Hide resolved
@tsalo
Copy link
Member Author

tsalo commented Nov 12, 2024

Something went wrong with the filename patterns for desc-slice_qc. I'll look into it tomorrow.

@tsalo
Copy link
Member Author

tsalo commented Nov 13, 2024

I always assumed that the filename patterns in the DerivativesDataSink were selected based on the number of matching entities, but I guess the order of patterns in the list actually matters. Moving the "bad" pattern to the end of the list seems to fix the problem.

@tsalo tsalo requested a review from mattcieslak November 13, 2024 13:46
@tsalo
Copy link
Member Author

tsalo commented Nov 13, 2024

It's finally passing. @mattcieslak could you take one more look before we merge?

Copy link
Collaborator

@mattcieslak mattcieslak left a comment

Choose a reason for hiding this comment

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

I can add those small docs changes to a different PR

docs/preprocessing.rst Outdated Show resolved Hide resolved
docs/preprocessing.rst Outdated Show resolved Hide resolved
@tsalo
Copy link
Member Author

tsalo commented Nov 13, 2024

Thanks! I'll merge once CI passes then.

@tsalo tsalo merged commit 6a798a1 into master Nov 13, 2024
20 checks passed
@tsalo tsalo deleted the space-acpc branch November 13, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Issues/PRs that change results or interfaces.
Projects
None yet
3 participants