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] - Enable to automagically fill Sources key #211

Closed
wants to merge 3 commits into from

Conversation

SamGuay
Copy link
Member

@SamGuay SamGuay commented Jun 21, 2023

Following the new id method, I've improved how sidecarChanges work.
Instead of plainly parsing the value of sidecarChanges, it looks for the Sources key and then look for id to swap filename for.

Also possible to pass a plain string to Sources as well, it won't complain that there are no match with id as it happens with the intendedFor key atm.

Very useful when dealing with MP2RAGE data (ie UNIT1 file), see this section of the BIDS spec.

Shall some logging be added to inform the user about the changes happening within the sidecar file?

@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2023

Codecov Report

Merging #211 (7b97e42) into master (86f89e3) will decrease coverage by 0.92%.
The diff coverage is 52.94%.

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
- Coverage   80.07%   79.16%   -0.92%     
==========================================
  Files          15       15              
  Lines         833      835       +2     
  Branches      137      138       +1     
==========================================
- Hits          667      661       -6     
- Misses        134      141       +7     
- Partials       32       33       +1     
Flag Coverage Δ
pytest 79.16% <52.94%> (-0.92%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dcm2bids/utils/utils.py 79.31% <ø> (ø)
dcm2bids/acquisition.py 91.66% <52.94%> (-6.64%) ⬇️

@SamGuay SamGuay force-pushed the enh_sidecar-changes branch 2 times, most recently from 40cc67a to a9cd29a Compare June 23, 2023 02:20
SamGuay added 2 commits June 22, 2023 22:50
Prevents silently ignoring strings that dont match with an id.
Returns a single list with all values instead.
@SamGuay SamGuay force-pushed the enh_sidecar-changes branch from a9cd29a to 40553f0 Compare June 23, 2023 02:50
Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

What will happen if the key doesn't exist (typo for example) ?
Quick pep8.

@SamGuay
Copy link
Member Author

SamGuay commented Jun 23, 2023

What will happen if the key doesn't exist (typo for example) ?
If there is a typo (or no id associated with the (faulty) string, it will be parse as a string. So if someone needs to add something else, it will be pass along as a string (or dict, etc.)

@arnaudbore arnaudbore closed this Jun 23, 2023
@SamGuay SamGuay deleted the enh_sidecar-changes branch August 27, 2024 16:40
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