Skip to content

Adopt new DICOM study importer in the MRI pipeline #1264

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

Merged

Conversation

maximemulder
Copy link
Contributor

@maximemulder maximemulder commented Apr 4, 2025

Title. Replace the uses of dicomTar.pl by import_dicom_study.py in the LORIS MRI pipeline. I would prefer to have this merged for LORIS-MRI 27, but if someone has problems with it 28 is ok too.

Succesfully tested on my local LORIS VM.

@maximemulder maximemulder added the Difficulty: Simple PR or issue that should be easy to implement, review, or test label Apr 4, 2025
@maximemulder maximemulder added this to the 27.0 milestone Apr 4, 2025
@cmadjar
Copy link
Collaborator

cmadjar commented Apr 8, 2025

@maximemulder could you add this to the LORIS-MRI agenda to make sure everyone is onboard with changing this? I am definitely ok with it but I'd like to be sure it would be ok for CCNA and IBIS or any other projects.

@maximemulder
Copy link
Contributor Author

maximemulder commented Apr 14, 2025

@cmadjar I made the DICOM study importer a configuration option, added a depreciation warning, and made an SQL patch in LORIS#9749.

Hide whitespace changes for easier review since my autotrimmer removed some.

Copy link
Collaborator

@cmadjar cmadjar left a comment

Choose a reason for hiding this comment

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

@maximemulder Could you run the following script to update the perdoc MD files?

mass_perldoc_md_creation.pl -profile prod

@maximemulder
Copy link
Contributor Author

@cmadjar Updated ! It generated more than I thought because of whitespace difference but taking that into account, the result is pretty much what was expected.

@cmadjar cmadjar modified the milestone: 27.0 Apr 17, 2025
@cmadjar cmadjar merged commit 05e8e28 into aces:main Apr 24, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Simple PR or issue that should be easy to implement, review, or test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants