poldracklab / pydeface

defacing utility for MRI images
MIT License
110 stars 42 forks source link

[feature proposal] BIDS directory as input #52

Open neurorepro opened 1 year ago

neurorepro commented 1 year ago

Hello,

Would it be of interest to add a feature so that pydeface accepts a BIDS directory as input, to make pydeface potentially run as a BIDS app ?

Many implementations are possible, with one possible first iteration e.g. relying on pybids and involving as parameters:

Best practice seems to be replacing the original file with the defaced one so the output could be:

poldrack commented 1 year ago

sounds like a great idea to me! probably need to think about how to handle cases where there is already a _desc- flag.

ofgulban commented 1 year ago

@neurorepro feel free to do a pull request. If you decide to do so, please ensure that your changes do not alter the current default behavior.

neurorepro commented 1 year ago

@ofgulban Sure, I will work on it and do a PR (everything backward compatible of course !)

ofgulban commented 1 year ago

saving the defaced file with the name of the original one (or e.g. ..._desc-defaced_...<suffix>.nii.gz)

This seems to have been discussed at #36 , maybe have a look there.

neurorepro commented 1 year ago

@ofgulban It could be temporarily included in the sidecar json as mentioned in #36.

Also a solution (hopefully soon fully BIDS compatbile) for the case @poldrack mentioned when a desc label is already present is to concatenate values with + as alluded to here

effigies commented 1 year ago

Note that https://github.com/PeerHerholz/BIDSonym wraps pydeface in a BIDS app that allows you to select your defacing method. Seems like it might be unnecessary effort building pydeface out into a BIDS app.

neurorepro commented 1 year ago

Hi @effigies , yes we saw that repo but it is no longer maintained

effigies commented 1 year ago

Oh. I was under the impression it was. cc @peerherholz for clarification.

neurorepro commented 1 year ago

I think it is also harder to maintain an aggregated tool depending on four defacing utilities than a single defacing utility.

By no longer maintained, i meant no actively maintained (the last commit is from 2021).

neurorepro commented 1 year ago

If it is deemed unnecessary to add BIDS app functionality to pydeface and @PeerHerholz's BIDSonym is actively maintained, then we will simply use BIDSonym

PeerHerholz commented 1 year ago

Ahoi hoi folks,

thx @effigies for the tag!

@neurorepro is definitely right in that BIDSonym is not really actively maintained, as I don't have time and funding to do so for a while now. The basic functionality is still working ok (I use the latest version quite often.) but it could for sure need a few updates here and there. I'm of course happy to support respective endeavors!

Cheers, Peer

neurorepro commented 1 year ago

Thank you @PeerHerholz for your feedback.

Then @effigies do you think it can be useful to add a BIDS app functionality to pydeface ?

effigies commented 1 year ago

Fine by me. I'm not maintaining here...

ofgulban commented 1 year ago

@neurorepro , looking at the latest commits, it seems that I am the one who is maintaining here. I would be curious to see a PR, but also at the same time worried if it is a big one (conference season is coming with work piling up). So my suggestion would be to maybe if possible start with something small but does what you wanted. I have to say that I am not familiar with the full extend of what "BIDS app functionality" means. I generally have two main worries:

So if you can address these, I would be happy to go through a PR.

neurorepro commented 1 year ago

@ofgulban sure, BIDS app functionality would mean having it BIDS-aware, i.e. run on a BIDS dataset without having to tell it where is the anatomical data (because BIDS does that)

ofgulban commented 1 year ago

@neurorepro that sounds cool and useful so that's fine with me.

Remi-Gau commented 1 year ago

am quickly trying to see if I can build a functional image of bidsonym that would have the latest version of each defacing tool... just in case this can avoid some wheel reinvention

Remi-Gau commented 1 year ago

I did not get super far but in case someone has some ideas, I am leaving this in a PR: https://github.com/PeerHerholz/BIDSonym/pull/70