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

future-proof BidsGuess #739

Closed
yarikoptic opened this issue Aug 9, 2023 · 2 comments
Closed

future-proof BidsGuess #739

yarikoptic opened this issue Aug 9, 2023 · 2 comments

Comments

@yarikoptic
Copy link
Contributor

yarikoptic commented Aug 9, 2023

why not to have instead of

"BidsGuess": ["anat","_acq-tse2_run-3_PDw"],

a

"BidsGuess": {
        "datatype": "anat",
        "filename_suffix": "_acq-tse2_run-3_PDw",
        "entities": {"acq": "tse2", "run": "3"},
        "suffix": "PDw"
    },

so it would make it easier and more robust for underlying tools, happen you decide to e.g. add some extra metadata field (e.g. "subject" which is available in DICOM or "accession" to be used for a session happen someone desires).

"filename_suffix" is just a convenience composition from "entities" and "suffix". Having those entities separate would facilitate reuse (e.g. in heudiconv) without adding ad-hoc splitting logic.

@neurolabusc
Copy link
Collaborator

This issue refers to the experimental BidsGuess feature found in the latest development branch of dcm2niix. I certainly appreciate community input, and am happy to modify the behavior if there is a consensus in the community.

One thing I do not like with your suggestions it redundancy which can lead to conflict (e.g. what gets precedence if items in the filename_suffix do not match items in entities or suffix list. A feature I like with the current implementation is that it makes the order of items very clear. BIDS and the bids-validator are very specific regarding not only the presence and absence of entities, but also their order.

I do think a couple of lines of Python will parse the values you want from the current structure:

import json
jsonFile = open('bids.json', 'r')
values = json.load(jsonFile)
jsonFile.close()
datatype = values['BidsGuess'][0]
str = values['BidsGuess'][1].split("_")
str.remove("")
suffix = str.pop()
entities = dict(s.split('-') for s in str)
print(suffix)
print(entities)

@yarikoptic
Copy link
Contributor Author

(e.g. what gets precedence if items in the filename_suffix do not match items in entities or suffix list

sure thing the order in which bids2niix would populate them must follow BIDS standard order of entities . Indeed , probably (didn't check), order of items in JSON dict is not guaranteed, but python dicts now retain order, so should be ok there. And users would be able to use ready-to-use filename_suffix with correct order anyways.

I agree that there is redundancy (and I personally the first on the forefront of fighting duplication!) but here it is "machine produced" duplication in favor of facilitating easier use. Sure thing it is easy to parse it -- it indeed takes only so few lines of python. But with any line of code we are opening chances for it to get broken. E.g. in your example, you would need to use entities = dict(s.split('-', 1) for s in str) to possibly future proof for change of semantic in BIDS to allow labels to contain - (bids-standard/bids-specification#1165). If so happens, every custom code piece like that would need to be changed. Thus, in my proposal I aimed to provide ready to be used components at the minor cost of redundancy.

Anyways -- it was just my recommendation. Feel free to ignore etc.

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

No branches or pull requests

2 participants