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.

Sign up for GitHub

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

Jump to bottom

Add ProjectID to scanner candidate #914

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Add ProjectID to scanner candidate #914

merged 2 commits into from
Apr 20, 2023

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Apr 12, 2023

No description provided.

@laemtl laemtl requested review from cmadjar and regisoc April 12, 2023 20:03
cmadjar
cmadjar requested changes Apr 13, 2023
View reviewed changes
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.

@laemtl Noticed on little mistake in the commented code of MRI.pm. Once resolved, need to recreate the MRI.md file.

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm Show resolved Hide resolved
@laemtl laemtl requested a review from cmadjar April 13, 2023 14:49
@regisoc
Copy link
Contributor

regisoc commented Apr 14, 2023

Tested and having this error with imaging_upload_file.pl or tarchiveLoader.pl:

Can't locate DICOM/DICOM.pm in @INC (you may need to install the DICOM::DICOM module) (@INC contains: /opt/Loris-MRI/bin/mri/uploadNeuroDB /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /opt/Loris-MRI/bin/mri/uploadNeuroDB/NeuroDB/MRI.pm line 49.
BEGIN failed--compilation aborted at /opt/Loris-MRI/bin/mri/uploadNeuroDB/NeuroDB/MRI.pm line 49.
Compilation failed in require at /opt/Loris-MRI/bin/mri/uploadNeuroDB/NeuroDB/ImagingUpload.pm line 57.
BEGIN failed--compilation aborted at /opt/Loris-MRI/bin/mri/uploadNeuroDB/NeuroDB/ImagingUpload.pm line 57.
Compilation failed in require at uploadNeuroDB/imaging_upload_file.pl line 60.
BEGIN failed--compilation aborted at uploadNeuroDB/imaging_upload_file.pl line 60.

@cmadjar
Copy link
Collaborator

cmadjar commented Apr 14, 2023

@regisoc did you source the environment file before running the script? That could explain the error if the file was not sourced before executing the script

regisoc
regisoc approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@regisoc regisoc left a comment

Choose a reason for hiding this comment

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

I can see the Tarchive in the db now! LGTM, thanks @laemtl!

@laemtl laemtl mentioned this pull request Apr 18, 2023
Closed
cmadjar
cmadjar requested changes Apr 18, 2023
View reviewed changes
dicom-archive/profileTemplate.pl Outdated
@@ -33,7 +33,7 @@
# extracts the subject and timepoint identifiers from the patient name
# assumes identifers are stored as <PSCID>_<DCCID>_<visit> in PatientName field, where <visit> is 3 digits.
sub getSubjectIDs {
my ($patientName, $patientID, $scannerID, $dbhr, $db) = @_;
my ($patientName, $scannerID, $dbhr, $db) = @_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't removing the patientID field in the prod file function getSubjectIDs break some projects that might be using DICOM header PatientID instead of PatientName to determine subject IDs? Unless the value $patientName is either PatientName header or PatientID header based on the lookupCenterName config value?

@nicolasbrossard thoughts on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not necessary if it breaks things. I just noticed it was not used in the default logic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK then, if you can revert this specific change just in case there is a project that is using PatientID in their prod file. Thanks!

@cmadjar cmadjar added this to the 25.0.0 milestone Apr 18, 2023
@cmadjar cmadjar merged commit 033d150 into aces:main Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@regisoc regisoc regisoc approved these changes

@cmadjar cmadjar cmadjar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@laemtl @regisoc @cmadjar