search

DCAN-Labs / dcan_bold_processing

functional connectivity preprocessing for resting state fMRI outputs of the dcan-fmri-pipelines
BSD 3-Clause "New" or "Revised" License
7 stars 7 forks source link

Hard-coded version number #18

Closed paul-reiners closed 1 year ago

paul-reiners commented 3 years ago

https://github.com/DCAN-Labs/dcan_bold_processing/blob/2ef154c6bc70dc3029d4b5941d1bf51f65db7334/dcan_bold_proc.py#L4

Does this need to be hard-coded? If so, could explanation be added to the code, including other modules where it might need to be changed?

perronea commented 2 years ago

It actually does need to be hard coded. We should be updating it when we release a new tagged version. I'm not sure how difficult it would be to go back and edit this field in the previous tags. I think we just need to make sure that we updated this field in new releases from now on.

Thank you for bringing this to our attention Paul.

ericearl commented 2 years ago

@perronea @paul-reiners @madisoth @arueter1 @ericfeczko I should warn you that the historical reason we didn't update this hard-coding was because it goes into file names which are taken as outputs and Kathy and I were afraid if we updated that string there it would break post-processing scripts/analyses that expected those filenames. So, I guess that's just a word of warning. I agree it should update with each version, but it's a risky change for downstream stuff.

perronea commented 2 years ago

That's a great point @ericearl. Maybe it should be removed all together and tracked in a separate metadata file within each subject's DBP directory just like the matlab config.

ericearl commented 2 years ago

@perronea I think that would be a better change, yeah. But certainly be careful about filename overlaps and downstream tools expecting the original version string.

LuciMoore commented 1 year ago

I'm closing this issue - as Anders explained, this does need to be hard-coded