Pet2bids (dicom only)

[bendhouseart]

The goals of this PR are very minimal:

• [x] Configure bidscoin such that if the pet2bids plugin is installed use it for PET files instead of anything else

• [x] Convert only dicom PET files at the moment (it's easy enough to update the plugin to target ecat and xlsx after the fact)

• [x] Minimally modify any code outside of the plugin

• [x] It should be noted that I did start this before bidscoin was using pytest so I need to refactor some of my tests to pytest instead

• More than happy to take up best BIDSCoin practices and modify to suite; at the moment I'm just tickled that this creates a bidsmap in the manner I intended.

• [x] Pass metadata from bidsmap to output from pet2bids (currently it gets lost in translation)

• [x] Update pet2bids to output the whole file name as destination, it's own logic is only extracting the sub- and ses- name from the target provided by bidscoin.

[marcelzwiers]

I can merge if you can fix the test? :-)

[bendhouseart]

I can merge if you can fix the test? :-)

Will refactor from unittest to pytest today and see if I can get tests passing. I'll convert this from draft when it's ready ready to merge. Still need to mint those bidcoins.

[bendhouseart]

Alright, I managed to get this coining with my lonely test subject 003. I think I'm going to need to do some slight refactoring on PET2BIDS (unless I've just forgotten how to use it) to make sure the filename's passed from BIDSCoin end up in the output. The same with the metadata collected from the bidsmapper, I'm not too surprised that some it isn't getting picked up as we're both shuffling our intermediary files through temporary directories.

However, it should be as easy as passing all of the BIDS metadata collected from the bidsmapper as key pair arguments to the dcm2niix4pet call.

[bendhouseart]

Added all tests to CI and installed dcm2niix on test runner. I had to do some light refactoring with the installation of spect2nii and pydeface by adding them to the requirements.txt file. Additionally, added a Makefile so installation and setup is the same on development machines and github's CI.

Not sure why pip install -e .[all] fails on Mac, but I think the changes I added fix it.

Either way, all of your tests now pass, so I can get back to breaking things on my end with the knowledge that the CI and make test will now scream at me if I mess up too bad.

[bendhouseart]

Oh, how do you pick up dcm2niix when it's on windows? Do you just check for it on the Path var?

[marcelzwiers]

. The same with the metadata collected from the bidsmapper, I'm not too surprised that some it isn't getting picked up as we're both shuffling our intermediary files through temporary directories.

Do you mean BIDScoin's unpacking of zipped files and/or DICOMDIR data? Otherwise, I think nothing is done in tempdirs?

However, it should be as easy as passing all of the BIDS metadata collected from the bidsmapper as key pair arguments to the dcm2niix4pet call.

BIDScoin has 3 ways of passing metadata: 1) Sidecar files made from the header (such as the JSONs produced by dcm2niix) 2) The sidecar file that is next to the source data (put there by the user beforehand) 3) The metadata entered by the user in the bidseditor

Is (2) something you can work with? I've never used it myself, so you can adapt it (see bids.copymetadata()) to your needs (and hence make it a generic mechanism throughout BIDScoin)

[marcelzwiers]

I had to do some light refactoring with the installation of spect2nii and pydeface by adding them to the requirements.txt file. Not sure why pip install -e .[all] fails on Mac, but I think the changes I added fix it.

Strange, I'll ask my colleague if he can test that too. But if the requirements.txt is refactored like this, then we also need to refactor setup.py alike (it's a minor detail, I know). Or even better, move over to pyproject.toml.

Additionally, added a Makefile so installation and setup is the same on development machines and github's CI

Great, I am still a CI newbie and had trouble figuring out how to configure github Actions. I tried installing dcm2niix there but failed...

[marcelzwiers]

Oh, how do you pick up dcm2niix when it's on windows? Do you just check for it on the Path var?

Well, I don't, I found that to be a wasp nest and I let the user set the path (in the bidsmap, see here). In the dccn we use: command: module add dcm2niix; dcm2niix

[marcelzwiers]

p.s. I think dcm2niix4petbids is not the best name for the plugin, how about renaming it to just pet2bids or pet2bidscoin?

[bendhouseart]

He sorry about falling behind on this, have been sick all week. Need to re-sync changes from master and finish off the last 2 check marks still.

To follow up:

Do you mean BIDScoin's unpacking of zipped files and/or DICOMDIR data? Otherwise, I think nothing is done in tempdirs?

I was referring to this sort of behavior:

INFO - dicomsort | >> Sorting: /var/folders/5t/0cjjt4t10hz5_qk4tts9gkv40000gs/T/tmp70tgnxfk/Users/galassiae/Projects/bidscoin/bidscointutorial/raw/sub-003/ses-01 (127 files)
INFO - dicomsort |    Creating:  /var/folders/5t/0cjjt4t10hz5_qk4tts9gkv40000gs/T/tmp70tgnxfk/Users/galassiae/Projects/bidscoin/bidscointutorial/raw/sub-003/ses-01/30003-Phantom_PetAcquisition_AC Images```

[bendhouseart]

BIDScoin has 3 ways of passing metadata: 1) Sidecar files made from the header (such as the JSONs produced by dcm2niix) 2) The sidecar file that is next to the source data (put there by the user beforehand) 3) The metadata entered by the user in the bidseditor

Is (2) something you can work with? I've never used it myself, so you can adapt it (see bids.copymetadata()) to your needs (and hence make it a generic mechanism throughout BIDScoin)

Using 3 was my primary plan and then sending that metadata to the subprocess.run call for pet2bids via: dcm2niix4pet < input > -d < output > --kwargs MetadataOne=[0, 1, 2, 3] TimeZero='12:12:12'

I'll see what all bids.copy_metadata does and plan on passing any thing collected from that to the call above via the --kwargs flag.

[bendhouseart]

Strange, I'll ask my colleague if he can test that too. But if the requirements.txt is refactored like this, then we also need to refactor setup.py alike (it's a minor detail, I know). Or even better, move over to pyproject.toml.

Yeah, I only noticed it on Mac, it behaves fine elsewhere. And a pyproject.toml is the way to go, Poetry too even. It handles packaging so much better than I'm able to with setup.py and all that other wizardry. Everytime I've learned how to package stuff via python I've just as quickly forgotten it... So I just stick with poetry build and poetry publish for anything I package and send to pypi nowadays.

Great, I am still a CI newbie and had trouble figuring out how to configure github Actions. I tried installing dcm2niix there but failed...

Me too! Many times infact. We'll have to update the dcm2niix release uri in the CI as new versions of Dcm2niix are released, but it should yell at us as things break so I'm considering it 'solved' for now. I'm sure that process could be better automated, but I just haven't bothered to write a script to do it, yet.....

[bendhouseart]

Well, I don't, I found that to be a wasp nest and I let the user set the path (in the bidsmap, see here). In the dccn we use: command: module add dcm2niix; dcm2niix

Okay, we do something similar where if we're unable to find it on the system path we ask the user to set it in a config file. I'm sure I'll remember why I asked this once I enable the the CI to run on windows ;)

p.s. I think dcm2niix4petbids is not the best name for the plugin, how about renaming it to just pet2bids or pet2bidscoin?

You make a very valid point here.

[marcelzwiers]

I was referring to this sort of behavior:

INFO - dicomsort | >> Sorting: /var/folders/5t/0cjjt4t10hz5_qk4tts9gkv40000gs/T/tmp70tgnxfk/Users/galassiae/Projects/bidscoin/bidscointutorial/raw/sub-003/ses-01 (127 files)
INFO - dicomsort |    Creating:  /var/folders/5t/0cjjt4t10hz5_qk4tts9gkv40000gs/T/tmp70tgnxfk/Users/galassiae/Projects/bidscoin/bidscointutorial/raw/sub-003/ses-01/30003-Phantom_PetAcquisition_AC Images```

Oh, but that's because it's tutorial data, in there I did some unsorting for subject 2 as an exercise / demo. But you are right, extended attributes are not working for data that still needs preprocessing before it can be used by BIDScoin directly (this pre-processing is what unpack/dicomsort is doing)

[marcelzwiers]

Everytime I've learned how to package stuff via python I've just as quickly forgotten it... So I just stick with poetry build and poetry publish for anything I package and send to pypi nowadays.

I'd like to move over, it's just that I didn't have to time/energy to port things over from something that I don't fully understand to something else I understand even less :-)

[marcelzwiers]

Sorry, I noticed that I didn't properly implement the extended attributes in the DataSource class (I stored it rigidly in self.metadata). Will fix it soon

[marcelzwiers]

I have updated the DataSource class, the extended attributes are now a class method (i.e. DataSource._extattributes()). I'll update the docs as well, but here is already some explanation about how things work.

Adding a json-file to the source data extends the source attributes / gives the exact same results and behaviour as if one would have (over)written that data to (the header of) the source data file itself. So you can do some pre-processing and put json's before running bidscoin -- that data will be transparently available as normal attributes throughout bidscoin.

You can do many nice things now. For instance, you can use these (new or overruling) attributes in a template bidsmap to identify source datatypes (e.g. {ContrastAgent: .*Gado.*} if you have {ContrastAgent: PureGadolinium} in your source json).

Here's another thing you can do. To copy over one or more of the extended attributes to the BIDS sidecar (metadata) files, you can simply put their <<fieldname(s)>> in the meta-table -- either in a template bidsmap (then appears in the study bidsmap by default) or in the bidseditor (then the user can/has to add it). For the above example, you can put {ContrastName: <<ContrastAgent>>} in the meta table (it will appear as {ContrastName: PureGadolinium} in the BIDS sidecar file). I think this is the route you wanted to take.

NB: The <<>> dynamic brackets make that the value is not static but extracted only until bidscoiner runtime (single dynamic brackets <ContrastAgent> string is replaced by PureGadolinium already in the bidseditor / study bidsmap). The double dynamic brackets are needed when the value varies from subject to subject (think <<InjectedMass>>).

However, if you have a long list of extended attributes it can be cumbersome to put them all in the metadata table (even though you can right-click to import data there). This is where the copymetadata() function comes in (which you were talking about earlier). This is meant to be used by the plugins during bidscoiner runtime, and just transfers all extra source metadata to the BIDS folder (for each plugin, the user can control this if needed in the bidseditor by setting the allowed file extensions of the extra source metadata files). A scenario where copymetadata is very useful is e.g. when you have a non-BIDS nifti dataset that was created using dcm2niix and now you want to convert that to BIDS (in that case you just want to copy over all the metadata in the json files). The nibabel2bids plugin can do exactly that. NB: copymetadata is still a very rudimentary function (I never needed it myself so I didn't develop it very much)

If a field is present in the metadata table and also in the metadata transferred by copymetadata, then the metadata table should take precedence (this should be implemented as such in the plugin), so that the user gets what he sees in the bidseditor (WYSIWYG).

Hope this helps :-)

[bendhouseart]

Sorry for the delay on this and the late reply, I've been completely under the weather for the past 2 weeks or so.

I'm going to get the CI working as the dbca959 seems to fail there now, but that may just be my own doing merging in your latest changes from master on Donders-Institute/bidscoin.

That's what I get for pushing before running pytest tests/

[marcelzwiers]

I made some suggestions and accidentally pushed them :-)

What do you think? (see my commit)

[bendhouseart]

I made some suggestions and accidentally pushed them :-)

What do you think? (see my commit)

I think this works just dandy for Dicoms. I need to double check and see that we're collecting all the required fields in the bids editor, but otherwise it does what I think it should.

Do you think we should add in ECAT's and Spreadsheets or merge this then make smaller PR's for the other formats?

[marcelzwiers]

I think we can merge it in and do ECAT in a follow-up PR

[bendhouseart]

Groovy! Thanks!

[marcelzwiers]

There have been a lot of improvements since v3.7.4, so I am thinking of releasing a new version. Instead of going to v3.8.0, I might jump to v4.0, given that amongst others it is now under CI, there are new bidsapps and there is a new PET plugin :-). I'd like to align it though, with your timeline/plan for getting ECAT support ready. I can wait a bit more, but if it's going to take a while then I may keep that for v4.1?

[marcelzwiers]

p.s. If you have ideas on architectural changes, this is the time :-)