diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index ec58fae9f..7822a306a 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -31,7 +31,12 @@ jobs: uses: actions/checkout@v4 - name: Clone the LORIS core repository - run: git clone https://github.com/aces/Loris.git ./test/Loris + # Use the corresponding LORIS branch for integration tests. + # The branch name is either the target branch for PRs, or the current branch otherwise. + # Only copy the current state of the repository, the history is not needed for CI. + run: | + BRANCH_NAME="${{ github.base_ref || github.ref_name }}" + git clone --depth 1 --single-branch --branch "$BRANCH_NAME" https://github.com/aces/Loris.git ./test/Loris - name: Overwrite Raisinbread SQL files run: cp -f ./test/RB_SQL/*.sql ./test/Loris/raisinbread/RB_files/ diff --git a/docs/scripts_md/MRI.md b/docs/scripts_md/MRI.md index e652b4ace..4aca1a8ee 100644 --- a/docs/scripts_md/MRI.md +++ b/docs/scripts_md/MRI.md @@ -231,19 +231,17 @@ RETURNS: `CandID` (int) ### getPSC($patientName, $dbhr, $db) -Looks for the site alias using the `session` table `CenterID` as -a first resource, for the cases where it is created using the front-end, -otherwise, find the site alias in whatever field (usually `patient_name` -or `patient_id`) is provided, and return the `MRI_alias` and `CenterID`. +Looks for the `CenterID` of a scan. If the configuration already provides a +`CenterID`, it is validated against the `psc` table. Otherwise, the `CenterID` +is looked up using the `session` table, and as a last resort by matching the site alias against whatever field +(usually `patient_name` or `patient_id`) is provided. INPUTS: - $patientName: patient name - $dbhr : database handle reference - $db : database object -RETURNS: a two element array: - - first is the MRI alias of the PSC or "UNKN" - - second is the `CenterID` or 0 +RETURNS: the `CenterID` or 0 if no center could be found ### getProject($subjectIDsref, $dbhr, $db) diff --git a/docs/scripts_md/delete_imaging_upload.md b/docs/scripts_md/delete_imaging_upload.md index e8512abed..262338a7e 100644 --- a/docs/scripts_md/delete_imaging_upload.md +++ b/docs/scripts_md/delete_imaging_upload.md @@ -478,20 +478,6 @@ from the file system and the database, using `gzip`. INPUTS: - $backupPath: path of the backup file to compress (without the .tar.gz extension). -### updateSessionTable($dbh, $mriUploadsRef, $tmpSQLFile) - -Sets to `N` the `Scan_done` column of all `sessions` in the database that do not have an associated upload -after the script has deleted those whose IDs are passed on the command line. The script also adds an SQL statement -in the SQL file whose path is passed as argument to restore the state that the `session` table had before the deletions. - -INPUTS: - - $dbh : database handle. - - $mriUploadsRef: reference on an array of hashes containing the uploads to delete. Accessed like this: - `$mriUploadsRef->[0]->{'TarchiveID'}`(this would return the `TarchiveID` of the first `mri_upload` - in the array. The properties stored for each hash are: `UploadID`, `TarchiveID`, `FullPath` - `Inserting`, `InsertionComplete` and `SessionID`. - - $tmpSQLFile: path of the SQL file that contains the SQL statements used to restore the deleted records. - ### updateFilesIntermediaryTable($dbh, $filesRef, $tmpSQLFile) Sets the `TarchiveSource` and `SourceFileID` columns of all the defaced files to `$tarchiveID` and `NULL` diff --git a/docs/test_plans/perl/delete_imaging_upload.md b/docs/test_plans/perl/delete_imaging_upload.md new file mode 100644 index 000000000..3c5cbc6c3 --- /dev/null +++ b/docs/test_plans/perl/delete_imaging_upload.md @@ -0,0 +1,11 @@ +# Test plan for `delete_imaging_upload.pl` + +## Manual tests + +- [ ] Test deletion of an entire MRI upload (UploadID) with SQL and file backup (ensure the SQL and file backup repopulate the database correctly after the deletion) +- [ ] Test deletion of an UploadID that has the same TarchiveID as an other UploadID - this should fail with an error message explaining there are multiple UploadID for a given TarchiveID +- [ ] Test deletion of an UploadID with QC information attached - this should fail with proper error message +- [ ] Test deletion of an UploadID with a parameter_form filled and the option `-form` and ensure the entries in the parameter form have been deleted +- [ ] On a defaced dataset, test running the delete script with the `-defaced` option - this should delete only non-defaced MINC files for that UploadID +- [ ] Run the delete script with option `-basename ` specifying a basename for the MINC files to be deleted - only images matching that basename should be deleted for the UploadID + diff --git a/docs/test_plans/perl/dicomTar.md b/docs/test_plans/perl/dicomTar.md new file mode 100644 index 000000000..b6f689175 --- /dev/null +++ b/docs/test_plans/perl/dicomTar.md @@ -0,0 +1,8 @@ +# Test plan for `dicomTar.pl` + +## Manual tests + +- Run `dicomTar.pl` without the `-database` flag and ensure nothing gets inserted into the DB. +- Run `dicomTar.pl` with the `-database` flag and check that entries got inserted into the DB in the `tarchive*` tables correctly. +- Run `dicomTar.pl` with the `-mri_upload_update` flag on a DICOM directory that was not previously uploaded via the imaging uploader (ensure there is nothing in `mri_upload` table for that study before running the script) => ensure that an entry has been added to `mri_upload` for the uploaded DICOM study. +- Run `updateMri_upload.pl` on a DICOM archive and ensure an MRI upload is created. diff --git a/docs/test_plans/perl/mass_nii.md b/docs/test_plans/perl/mass_nii.md new file mode 100644 index 000000000..3a484b7d9 --- /dev/null +++ b/docs/test_plans/perl/mass_nii.md @@ -0,0 +1,18 @@ +# Test plan for `mass_nii.pl` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` + +- run `mass_nii.pl -help` and ensure the help for the script gets printed + +- remove NIfTI images for UploadID 106 (linked to TarchiveID 56) + - remove pics from the filesystem using `rm /data/Loris-MRI/data/assembly/400168/V2/mri/native/*nii /data/Loris-MRI/data/assembly/400168/V2/mri/native/*bval /data/Loris-MRI/data/assembly/400168/V2/mri/native/*bvec` + - in MySQL run the following query to delete path to pic images in `parameter_file`: `DELETE parameter_file FROM parameter_file JOIN files USING (FileID) WHERE TarchiveSource=56 AND Value like "%nii";` (should delete 5 rows) + - check that the following query returns no results: `SELECT FileID, Value FROM parameter_file JOIN files USING (FileID) WHERE TarchiveSource=56 AND Value like "%nii";` + - go to the imaging browser for CandID 400168 V2 and check that the buttons "download NIfTI" does not show up anymore under the image's screenshot" + +- run `mass_nii.pl` on `FileIDs` `335` to `339`: `mass_nii.pl -profile prod -minFileID 335 -maxFileID 339` + - check that the "download NIfTI" button shows up again below the images' screenshots. + - check that the following query returns 5 rows: `SELECT FileID, Value FROM parameter_file JOIN files USING (FileID) WHERE TarchiveSource=56 AND Value like "%nii.gz";` + - check that files with extension ".nii" or ".nii.gz" have been created under `/data/Loris-MRI/data/assembly/400168//V2/mri/native/` diff --git a/docs/test_plans/perl/tarchiveLoader.md b/docs/test_plans/perl/tarchiveLoader.md new file mode 100644 index 000000000..813ebae02 --- /dev/null +++ b/docs/test_plans/perl/tarchiveLoader.md @@ -0,0 +1,12 @@ +# Test plan for `tarchiveLoader.md` + +## Manual tests + +- [ ] Check that -uploadID, -profile and an existing tarchive are mandatory script argument. +- [ ] Ensure passing an invalid upload ID yields an error. +- [ ] Ensure passing an invalid tarchive (e.g csv file) yields an error. +- [ ] Check that if config setting get_dicom_info is not properly set, tarchiveLoader issues an error that the get_dicom_info command could not be run. +- [ ] Pick an archive that's already been successfully processed and find its associated upload ID. Run tarchiveLoader using this archive but use option -upload_id with an argument that is not its actual upload ID. Check that you get an error message that the archive and upload ID don't match. +- [ ] Pick an archive that's already been successfully processed and find its associated upload ID. Run tarchiveLoader using this archive and option -upload_id with an argument that is its actual upload ID. Ensure that no minc files are inserted and you get an error message regarding duplicate md5sum for each scan that was originally inserted when the archive was processed. +- [ ] Ensure that config setting default_project is not set and that your prod file does not define any PSCID, visitLabel and ProjectID in function getSubjectIDs. Run tarchiveLoader with a matching tarchive and upload ID and ensure that you get an error message saying that config setting default_project has to be set. + diff --git a/docs/test_plans/perl/tarchive_validation.md b/docs/test_plans/perl/tarchive_validation.md new file mode 100644 index 000000000..1cfac8d22 --- /dev/null +++ b/docs/test_plans/perl/tarchive_validation.md @@ -0,0 +1,12 @@ +# Test plan for `tarchive_validation.pl` + +- [ ] If not already done, source the `environment` file: `source /opt/Loris-MRI/bin/mri/environment` +- [ ] run `tarchive_validation.pl -help` and ensure the help for the script gets printed +- [ ] run the `tarchive_validation.pl` script on a valid archive (example UploadID 106) +``` +tarchive_validation.pl -profile prod -uploadID 106 /data/Loris-MRI/data/tarchive/2016/DCM_2016-08-19_ImagingUpload-18-26-C4Y94V.tar +``` +- [ ] test `tarchive_validation.pl` script on an invalid archive (example UploadID 109) + - [ ] delete file `/data/Loris-MRI/data/tarchive/2016/DCM_2016-08-15_ImagingUpload-18-34-hhQQY5.tar` + - [ ] recreate the file with `touch` command `touch /data/Loris-MRI/data/tarchive/2016/DCM_2016-08-15_ImagingUpload-18-34-hhQQY5.tar` + - [ ] run `tarchive_validation.pl -profile prod -uploadID 109 /data/Loris-MRI/data/tarchive/2016/DCM_2016-08-15_ImagingUpload-18-34-hhQQY5.tar` => this should fail with proper message due to different md5 diff --git a/docs/test_plans/python/import_bids_dataset.md b/docs/test_plans/python/import_bids_dataset.md new file mode 100644 index 000000000..ce0330592 --- /dev/null +++ b/docs/test_plans/python/import_bids_dataset.md @@ -0,0 +1,33 @@ +# LORIS BIDS importer test plan + +## Preamble + +- The LORIS BIDS importer is a script to import a BIDS dataset in LORIS. +- Take a look at the documentation in the `loris-bids-importer` package README file. + +## Testing instructions + +### General + +- Ensure that importing an incorrect path or non-BIDS file or directory returns an error. +- Ensure that the `--create-session` option creates a session in LORIS. +- Ensure that the `--create-candidate` option creates a candidate in LORIS. + +### MRI + +Import a BIDS dataset with MRI data. +- Ensure that the BIDS importer returns no unexpected error. +- Ensure that the imported MRI data is visible in the LORIS imaging browser, with brain browser visualization and preview pictures. +- Ensure that the imported MRI files are downloadlable in the LORIS imaging browser (NIfTI, sidecar JSON, BVAL and BVEC files) + +### EEG + +Import a BIDS dataset with EEG data. +- Ensure that the BIDS importer returns no unexpected error.. +- Ensure that the imported EEG data is visible in the LORIS electrophysiology browser. +- Ensure that having the `useEEGBrowserVisualizationComponents` LORIS configuration option set to `1` or `true` during the import creates the electrophysiliogy chunk files, which allows to visualize the EEG signals int the LORIS electrophysiology browser after the import. +- Ensure that the imported EEG files are downloadlable in the LORIS imaging browser (acquisition file, sidecar JSON, event files, archives). + +## Testing data + +There is no data readily available for import for now. You can find use the Raisinbread data in the LORIS data directories `assembly_bids` (MRI) and `bids_imports` (Face13 dataset, EEG) if you remove them from LORIS before testing. You are encouraged to use your own datasets (such as public datasets) if you have some. diff --git a/docs/test_plans/python/import_dicom_study.md b/docs/test_plans/python/import_dicom_study.md new file mode 100644 index 000000000..5fc53db9b --- /dev/null +++ b/docs/test_plans/python/import_dicom_study.md @@ -0,0 +1,49 @@ +# LORIS DICOM importer test plan + +## Preamble + +- The LORIS DICOM importer is a script to import a DICOM study in LORIS. +- The LORIS DICOM importer code and documentation is located in the `loris-bids-importer` package. +- It is advised to take a look at the package README file before starting the tests. +- The `tarchiveLibraryDir` LORIS configuration option should point to an existing directory before starting the tests. + +## Testing instructions + +### Running the script + +Ensure a DICOM study can be imported into LORIS using the `--insert` CLI option: +```sh +import-dicom-study --insert --session --source /path/to/dicom/study +``` + +### Imported DICOM study + +- Ensure the DICOM study is visible in the LORIS DICOM archive module, with a list of all its DICOM files and working download link. +- Ensure the DICOM study is attached to a LORIS session based on its patient identifiers (from the `--session` CLI option). +- Ensure the DICOM study is present in the LORIS database in the `tarchive_*` tables. +- Ensure the DICOM study archive is present in the LORIS `tarchive` directory. +- Ensure the DICOM study archive contains a `.log` and `.meta`file, and that both their contents look correct. + +### Other commands + +- Ensure that a DICOM study that is already imported in LORIS can be updated by using the `--update` and `--overwrite` CLI options instead of `--insert`. +- Ensure the `summarize-dicom-study` script produces a correct looking DICOM study summary. + +### Errors cases + +- Ensure that importing a non-DICOM study file or directory results in an error. +- Ensure that importing a DICOM study that is already in LORIS results in an error. +- Ensure that updating a DICOM study that is not already in LORIS results in an error. + +## Testing data + +Usable DICOM studies may be found in the `/data/loris/incoming` directory, as well as already archived DICOM studies in the `/data/loris/tarchive` directory (which should then be untarred for testing). + +To test the DICOM study importer with an already imported DICOM study, existing data should first be removed from the LORIS database and data directory. + +A DICOM study an be removed from the database using the following SQL statement: +```sql +DELETE FROM tarchive WHERE tarchiveID = @id; +``` + +If foreign keys exist in other tables (like `mri_upload` or `files`), it is advised to set the relevant attributes to `NULL`. diff --git a/docs/test_plans/python/mass_nifti_pic.md b/docs/test_plans/python/mass_nifti_pic.md new file mode 100644 index 000000000..79e643b94 --- /dev/null +++ b/docs/test_plans/python/mass_nifti_pic.md @@ -0,0 +1,18 @@ +# Test plan for `mass_nifti_pic.py` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` +- run `mass_nifti_pic.py -h` + => should print the help of the script. Make sure the help documentation is up-to-date. +- Bonus points: verify that the automated tests are still implemented + +## Automated tests already implemented + +- test invalid profile +- test smallest `FileID` bigger than largest `FileID` +- test invalid FileID provided +- test on a FileID that already has a pic +- test force option +- test running on a text file +- test successful run diff --git a/docs/test_plans/python/run_dicom_archive_loader.md b/docs/test_plans/python/run_dicom_archive_loader.md new file mode 100644 index 000000000..8a39793c6 --- /dev/null +++ b/docs/test_plans/python/run_dicom_archive_loader.md @@ -0,0 +1,20 @@ +# Test plan for `run_dicom_archive_loader.py` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` +- run `run_dicom_archive_loader.py -h` + => should print the help of the script. Make sure the help documentation is up-to-date. +- run `run_dicom_archive_loader.py -p config.py -u ` on a valid upload ID with fieldmaps and BOLD images + => ensure that the `IntendedFor` field of the fieldmap has been updated to include the path to the BOLD images + => once the script is done running, ensure that the temporary directory that was used to run the script has been cleared out +- run `run_dicom_archive_loader.py -p config.py -u -s ` + => ensure only that the file(s) matching the `SeriesUID` have been ingested +- Bonus points: verify that the automated tests are still implemented + +## Automated tests already implemented + +- test invalid argument +- test invalid upload ID +- test invalid tarchive path +- test successful run on valid tarchive path diff --git a/docs/test_plans/python/run_dicom_archive_validation.md b/docs/test_plans/python/run_dicom_archive_validation.md new file mode 100644 index 000000000..a7559d4db --- /dev/null +++ b/docs/test_plans/python/run_dicom_archive_validation.md @@ -0,0 +1,18 @@ +# Test plan for `run_dicom_archive_validation.py` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` +- run `run_dicom_archive_validation.py -h` + => should print the help of the script. Make sure the help documentation is up-to-date. +- Bonus points: verify that the automated tests are still implemented + +## Automated tests already implemented + +- test missing upload ID argument +- test missing tarchive path argument +- test invalid argument +- test invalid upload ID +- test invalid tarchive path +- test mixed up upload ID and tarchive path +- test successful validation diff --git a/docs/test_plans/python/run_nifti_insertion.md b/docs/test_plans/python/run_nifti_insertion.md new file mode 100644 index 000000000..8428b8446 --- /dev/null +++ b/docs/test_plans/python/run_nifti_insertion.md @@ -0,0 +1,25 @@ +# Test plan for `run_nifti_insertion.py` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` +- run `run_nifti_insertion.py -h` + => should print the help of the script. Make sure the help documentation is up-to-date. +- Bonus points: verify that the automated tests are still implemented + +## Automated tests already implemented + +- test invalid argument +- test missing NIfTI path argument +- test invalid NIfTI path +- test missing upload ID or tarchive path argument (one of them should be set) +- test missing JSON path argument +- test invalid JSON path +- test invalid upload ID +- test invalid tarchive path +- test tarchive path and upload ID argument provided (only one should be set) +- test NIfTI and tarchive `PatientName` differ +- test NIfTI already inserted +- test NIfTI MRI protocol violated scans features +- test NIfTI MRI violations log exclude features +- test DWI insertion with MRI violations warning \ No newline at end of file diff --git a/docs/test_plans/python/run_push_imaging_files_to_s3_pipeline.md b/docs/test_plans/python/run_push_imaging_files_to_s3_pipeline.md new file mode 100644 index 000000000..2ceecfe95 --- /dev/null +++ b/docs/test_plans/python/run_push_imaging_files_to_s3_pipeline.md @@ -0,0 +1,17 @@ +# Test plan for `run_dicom_archive_validation.py` + +## Manual tests + +- If not already done, source the environment file: `source /opt/Loris-MRI/bin/mri/environment` +- run `run_push_imaging_files_to_s3_pipeline.py -h` + => should print the help of the script. Make sure the help documentation is up-to-date. +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py -u ` on a valid Upload ID and + ensure the files has been properly pushed to the S3 bucket +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py` without the `-u` + => should print `[ERROR ] argument --upload_id is required` +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py -u ` on an invalid Upload ID + => should print `[ERROR ] Did not find an entry in mri_upload associated with 'UploadID' 1666` +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py -u ` with the S3 config settings not set in `config.py` +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py -u ` with incorrect S3 authentication settings in `config.py` +- run `run_push_imaging_files_to_s3_pipeline.py -p config.py -u ` with the incorrect S3 bucket name in `config.py` + diff --git a/install/imaging_install.sh b/install/imaging_install.sh index 0ef2e1e91..3058932f1 100755 --- a/install/imaging_install.sh +++ b/install/imaging_install.sh @@ -216,7 +216,7 @@ echo "Creating python config file with database credentials" cp $installdir/templates/config_template.py $mridir/config/config.py sudo chmod 640 $mridir/config/config.py sudo chgrp $group $mridir/config/config.py -sed -e "s#DBNAME#$mysqldb#g" -e "s#DBUSER#$mysqluser#g" -e "s#DBPASS#$mysqlpass#g" -e "s#DBHOST#$mysqlhost#g" $installdir/templates/database_config_template.py > $mridir/config/config.py +sed -e "s#DBNAME#$mysqldb#g" -e "s#DBUSER#$mysqluser#g" -e "s#DBPASS#$mysqlpass#g" -e "s#DBHOST#$mysqlhost#g" $installdir/templates/config_template.py > $mridir/config/config.py echo "config file for python import scripts is located at $mridir/config/config.py" echo diff --git a/install/imaging_install_MacOSX.sh b/install/imaging_install_MacOSX.sh index 70c38dc90..84bea02a7 100755 --- a/install/imaging_install_MacOSX.sh +++ b/install/imaging_install_MacOSX.sh @@ -128,6 +128,6 @@ echo "Creating python config file with database credentials" cp $installdir/templates/config_template.py $mridir/config/config.py sudo chmod 640 $mridir/config/config.py sudo chgrp $group $mridir/config/config.py -sed -e "s#DBNAME#$mysqldb#g" -e "s#DBUSER#$mysqluser#g" -e "s#DBPASS#$mysqlpass#g" -e "s#DBHOST#$mysqlhost#g" $installdir/templates/database_config_template.py > $mridir/config/config.py +sed -e "s#DBNAME#$mysqldb#g" -e "s#DBUSER#$mysqluser#g" -e "s#DBPASS#$mysqlpass#g" -e "s#DBHOST#$mysqlhost#g" $installdir/templates/config_template.py > $mridir/config/config.py echo "config file for python import scripts is located at $mridir/config/config.py" echo diff --git a/install/templates/environment_template b/install/templates/environment_template index cf3c93b3d..cd8360b4a 100644 --- a/install/templates/environment_template +++ b/install/templates/environment_template @@ -1,6 +1,10 @@ PROJECT=%PROJECT% MINC_TOOLKIT_DIR=%MINC_TOOLKIT_DIR% +# source the Python environment first so that other environment variables are added to this environment +export LORIS_MRI=/opt/${PROJECT}/bin/mri +source /opt/${PROJECT}/bin/mri/.venv/bin/activate + # to source the MINC toolkit source ${MINC_TOOLKIT_DIR}/minc-toolkit-config.sh umask 0002 @@ -11,10 +15,6 @@ export PERL5LIB=/opt/${PROJECT}/bin/mri/uploadNeuroDB:/opt/${PROJECT}/bin/mri/di export TMPDIR=/tmp export LORIS_CONFIG=/opt/${PROJECT}/bin/mri/config -# for the Python scripts -export LORIS_MRI=/opt/${PROJECT}/bin/mri -source /opt/${PROJECT}/bin/mri/.venv/bin/activate - # for the defacing scripts export BEASTLIB=${MINC_TOOLKIT_DIR}/../share/beast-library-1.1 export MNI_MODELS=${MINC_TOOLKIT_DIR}/../share/icbm152_model_09c diff --git a/pyproject.toml b/pyproject.toml index 0156c2b38..b38788022 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -135,6 +135,7 @@ locale = "en-us" Centre = "Centre" # McGill Centre for Integrative Neuroscience HED = "HED" # Hierarchical Event Descriptors ND = "ND" # Image Type DICOM attribute +Colour = "Colour" # SQL column name in sex table [tool.typos.default.extend-identifiers] ba = "ba" # A CLI option of dcm2niix diff --git a/python/lib/db/models/sex.py b/python/lib/db/models/sex.py index 3350a83ef..13109d7ee 100644 --- a/python/lib/db/models/sex.py +++ b/python/lib/db/models/sex.py @@ -6,4 +6,5 @@ class DbSex(Base): __tablename__ = 'sex' - name : Mapped[str] = mapped_column('Name', primary_key=True) + name : Mapped[str] = mapped_column('Name', primary_key=True) + color: Mapped[str | None] = mapped_column('Colour') diff --git a/python/lib/db/models/user.py b/python/lib/db/models/user.py index 07aa18664..edf87658a 100644 --- a/python/lib/db/models/user.py +++ b/python/lib/db/models/user.py @@ -4,8 +4,8 @@ import lib.db.models.project as db_project import lib.db.models.site as db_site -import lib.db.models.user_project # type: ignore # noqa: F401 -import lib.db.models.user_site # type: ignore # noqa: F401 +import lib.db.models.user_project # type: ignore # ruff:ignore[unused-import] +import lib.db.models.user_site # type: ignore # ruff:ignore[unused-import] from lib.db.base import Base from lib.db.decorators.int_bool import IntBool from lib.db.decorators.y_n_bool import YNBool diff --git a/python/lib/dcm2bids_imaging_pipeline_lib/push_imaging_files_to_s3_pipeline.py b/python/lib/dcm2bids_imaging_pipeline_lib/push_imaging_files_to_s3_pipeline.py index fa12248b0..1b1ca90d5 100644 --- a/python/lib/dcm2bids_imaging_pipeline_lib/push_imaging_files_to_s3_pipeline.py +++ b/python/lib/dcm2bids_imaging_pipeline_lib/push_imaging_files_to_s3_pipeline.py @@ -98,8 +98,8 @@ def _get_list_of_files_from_files(self): for file in self.dicom_archive.mri_files: # Get the raw path of that file, without converting it to a Python path object. - raw_path: str = inspect(file).attrs.path.loaded_value - if raw_path.startswith('s3://'): + raw_path: str = str(inspect(file).attrs.path.loaded_value) + if raw_path.startswith('s3:/'): # skip since file already pushed to S3 continue self.files_to_push_list.append({ @@ -271,6 +271,12 @@ def _clean_up_empty_folders(self): # remove empty folders from file system print("Cleaning up empty folders") - remove_empty_directories(self.data_dir / 'assembly_bids' / f'sub-{self.session.candidate.cand_id}') - remove_empty_directories(self.data_dir / 'pic' / str(self.session.candidate.cand_id)) - remove_empty_directories(self.data_dir / 'trashbin') + assembly_dir = self.data_dir / 'assembly_bids' / f'sub-{self.session.candidate.cand_id}' + pic_dir = self.data_dir / 'pic' / str(self.session.candidate.cand_id) + trashbin_dir = self.data_dir / 'trashbin' + if assembly_dir.exists(): + remove_empty_directories(assembly_dir) + if pic_dir.exists(): + remove_empty_directories(pic_dir) + if trashbin_dir.exists(): + remove_empty_directories(trashbin_dir) diff --git a/python/lib/env.py b/python/lib/env.py index b92913d99..2320ecaec 100644 --- a/python/lib/env.py +++ b/python/lib/env.py @@ -1,3 +1,4 @@ +import shutil from collections.abc import Callable from dataclasses import dataclass from pathlib import Path @@ -39,11 +40,28 @@ class Env: script_name: str config_info: Any tmp_dir_path: Path - log_file_path: Path + log_file_path: Path | None verbose: bool cleanups: list[Callable[[], None]] notifier: Notifier | None = None + def close(self): + """ + Close the environment resources and remove its temporary directory. It is not required to + manually call this method in processes that only have a single long-lived environment. But + it is advised to do so in processes that manage multiple short-lived environments. + """ + + try: + if self.notifier is not None: + self.notifier.db.close() + finally: + try: + self.db.close() + finally: + self.db_engine.dispose() + shutil.rmtree(self.tmp_dir_path, ignore_errors=True) + def add_cleanup(self, cleanup: Callable[[], None]): """ Add a cleanup function to the environment, which will be executed if the program exits diff --git a/python/lib/imaging_lib/file.py b/python/lib/imaging_lib/file.py index 9e5ca481e..861ab979b 100644 --- a/python/lib/imaging_lib/file.py +++ b/python/lib/imaging_lib/file.py @@ -1,5 +1,5 @@ import getpass -from datetime import date +from datetime import date, datetime from pathlib import Path from lib.db.models.dicom_archive import DbDicomArchive @@ -35,6 +35,7 @@ def register_mri_file( file_type = file_type.name, session_id = session.id, inserted_by_user_id = getpass.getuser(), + insert_time = datetime.now(), coordinate_space = 'native', output_type = 'native', series_uid = series_instance_uid, diff --git a/python/lib/imaging_lib/file_parameter.py b/python/lib/imaging_lib/file_parameter.py index 6fefe8fc7..f2bed6552 100644 --- a/python/lib/imaging_lib/file_parameter.py +++ b/python/lib/imaging_lib/file_parameter.py @@ -31,13 +31,11 @@ def register_mri_file_parameter(env: Env, file: DbFile, parameter_name: str, par parameter = try_get_file_parameter_with_file_id_type_id(env.db, file.id, parameter_type.id) if parameter is None: - time = datetime.now() - parameter = DbFileParameter( type_id = parameter_type.id, file_id = file.id, value = parameter_value, - insert_time = time, + insert_time = datetime.now(), ) env.db.add(parameter) diff --git a/python/lib/logging.py b/python/lib/logging.py index b28404a51..422df4c5a 100644 --- a/python/lib/logging.py +++ b/python/lib/logging.py @@ -65,6 +65,9 @@ def write_to_log_file(env: Env, message: str): Write a message to the log file of the environment. """ + if env.log_file_path is None: + return + with open(env.log_file_path, 'a') as file: file.write(f"{message}\n") diff --git a/python/lib/lorisgetopt.py b/python/lib/lorisgetopt.py index e2b1bb214..8399caf64 100644 --- a/python/lib/lorisgetopt.py +++ b/python/lib/lorisgetopt.py @@ -12,7 +12,7 @@ class LorisGetOpt: - # ruff: noqa + # ruff:ignore[doc-line-too-long] """ This class will handle GetOpt functions for scripts to be run. diff --git a/python/lib/make_env.py b/python/lib/make_env.py index 854f79b7c..47c0c9414 100644 --- a/python/lib/make_env.py +++ b/python/lib/make_env.py @@ -19,6 +19,7 @@ def make_env( script_options: dict[str, Any], config_info: Any, verbose: bool, + log_file: bool = True, ) -> Env: """ Create a new script environment using the provided arguments. @@ -51,10 +52,12 @@ def make_env( tmp_dir_path = create_script_tmp_dir(script_name) - log_dir_path = data_dir / 'logs' / script_name - log_dir_path.mkdir(exist_ok=True) - - log_file_path = log_dir_path / f'{tmp_dir_path.name}.log' + if log_file: + log_dir_path = data_dir / 'logs' / script_name + log_file_path = log_dir_path / f'{tmp_dir_path.name}.log' + log_dir_path.mkdir(exist_ok=True) + else: + log_file_path = None env = Env( engine, @@ -67,16 +70,17 @@ def make_env( [], ) - log_file_header = get_log_file_header(env, script_options) - write_to_log_file(env, log_file_header) + if env.log_file_path is not None: + log_file_header = get_log_file_header(env.log_file_path, script_options) + write_to_log_file(env, log_file_header) log_verbose(env, 'Successfully connected to the database') return env -def get_log_file_header(env: Env, script_options: dict[str, Any]): - run_info = env.log_file_path.name[:-13] +def get_log_file_header(log_file_path: Path, script_options: dict[str, Any]): + run_info = log_file_path.name[:-13] title = run_info.replace('_', ' ').upper() message = ( "\n" diff --git a/python/lib/physio/events.py b/python/lib/physio/events.py index 8b586a384..05a826d62 100644 --- a/python/lib/physio/events.py +++ b/python/lib/physio/events.py @@ -3,15 +3,15 @@ from pathlib import Path from typing import Any -from lib.db.models.physio_task_event_hed import DbPhysioTaskEventHed -from lib.db.models.physio_task_event_opt import DbPhysioTaskEventOpt -from lib.db.models.physio_task_event import DbPhysioTaskEvent -from lib.db.queries.hed_schema_node import get_all_hed_schema_nodes from lib.db.models.bids_event_dataset_mapping import DbBidsEventDatasetMapping from lib.db.models.bids_event_file_mapping import DbBidsEventFileMapping from lib.db.models.physio_event_file import DbPhysioEventFile from lib.db.models.physio_file import DbPhysioFile +from lib.db.models.physio_task_event import DbPhysioTaskEvent +from lib.db.models.physio_task_event_hed import DbPhysioTaskEventHed +from lib.db.models.physio_task_event_opt import DbPhysioTaskEventOpt from lib.db.models.project import DbProject +from lib.db.queries.hed_schema_node import get_all_hed_schema_nodes from lib.env import Env from lib.physio.hed import TagGroupMember, build_hed_tag_groups @@ -19,7 +19,8 @@ @dataclass class EventDictFileSource: """ - Class representing whether an event dictionary file is dataset-wide or comes from a specific acquisition. + Class representing whether an event dictionary file is dataset-wide or comes from a specific + acquisition. """ project: DbProject @@ -171,7 +172,8 @@ def parse_and_insert_event_dict( for event_name, event in event_dict.items(): tag_dict[event_name] = {} - # TODO: Commented fields below currently not supported # ruff: noqa + # ruff:ignore[doc-line-too-long] + # TODO: Commented fields below currently not supported # description = event_metadata[parameter]['Description'] \ # if 'Description' in event_metadata[parameter] \ # else None @@ -182,6 +184,7 @@ def parse_and_insert_event_dict( # value_hed = None else: is_categorical = 'N' + # ruff:ignore[doc-line-too-long] # value_hed = event_metadata[parameter]['HED'] if 'HED' in event_metadata[parameter] else None if is_categorical == 'Y': diff --git a/python/loris_bids_importer/src/loris_bids_importer/acquisitions.py b/python/loris_bids_importer/src/loris_bids_importer/acquisitions.py index 41a663772..1bc119b3d 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/acquisitions.py +++ b/python/loris_bids_importer/src/loris_bids_importer/acquisitions.py @@ -1,20 +1,60 @@ from collections.abc import Callable +from dataclasses import dataclass +from enum import Enum +from pathlib import Path from typing import TypeVar +from lib.db.models.session import DbSession from lib.env import Env from lib.logging import log, log_error from loris_bids_utils.info import BidsAcquisitionInfo +from loris_bids_importer.copy_files import add_bids_scan_row, get_loris_scans_path from loris_bids_importer.env import BidsImportEnv + +class BidsImportFileStatus(Enum): + """ + The status of a BIDS file import. + """ + + SUCCESS = 1 + """ + The file was successfully imported. + """ + + IGNORE = 2 + """ + The file was ignored, usually because it is already in LORIS. + """ + + +@dataclass +class BidsImportFileResult: + """ + The result of a BIDS acquisition import. + """ + + status: BidsImportFileStatus + """ + The status of the BIDS file import. + """ + + path: Path + """ + The path of the imported file relative to the LORIS data directory. + """ + + T = TypeVar('T') def import_bids_acquisitions( env: Env, import_env: BidsImportEnv, + session: DbSession, acquisitions: list[tuple[T, BidsAcquisitionInfo]], - importer: Callable[[T, BidsAcquisitionInfo], None] + importer: Callable[[T, BidsAcquisitionInfo], BidsImportFileResult] ): """ Run an import function on a list of BIDS acquisitions, logging the overall import progress, @@ -28,10 +68,22 @@ def import_bids_acquisitions( ) try: - importer(acquisition, bids_info) - log(env, f"Successfully imported acquisition '{bids_info.name}'.") - import_env.imported_acquisitions_count += 1 + result = importer(acquisition, bids_info) + match result.status: + case BidsImportFileStatus.SUCCESS: + # Update the LORIS scans.tsv file. + if bids_info.scans_file is not None and bids_info.scan_row is not None: + loris_scans_path = get_loris_scans_path(import_env, bids_info.scans_file, session) + bids_info.scan_row.set_file_name(result.path.name) + add_bids_scan_row(import_env, bids_info.scan_row, loris_scans_path) + + import_env.imported_acquisitions_count += 1 + log(env, f"Successfully imported acquisition '{bids_info.name}'.") + case BidsImportFileStatus.IGNORE: + import_env.ignored_acquisitions_count += 1 + log(env, f"File '{result.path}' is already registered in LORIS. Skipping.") except Exception as exception: + import_env.failed_acquisitions_count += 1 log_error( env, ( @@ -40,4 +92,3 @@ def import_bids_acquisitions( "Skipping." ) ) - import_env.failed_acquisitions_count += 1 diff --git a/python/loris_bids_importer/src/loris_bids_importer/copy_files.py b/python/loris_bids_importer/src/loris_bids_importer/copy_files.py index ac9f068b5..649741eda 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/copy_files.py +++ b/python/loris_bids_importer/src/loris_bids_importer/copy_files.py @@ -7,7 +7,7 @@ from lib.env import Env from loris_bids_utils.files.dataset_description import BidsDatasetDescriptionJsonFile from loris_bids_utils.files.participants import BidsParticipantsTsvFile -from loris_bids_utils.files.scans import BidsScansTsvFile +from loris_bids_utils.files.scans import BidsScansTsvFile, BidsScanTsvRow from loris_bids_importer.env import BidsImportEnv @@ -175,18 +175,20 @@ def copy_bids_participants_file( participants_file.write(participants_path) -def copy_bids_scans_file(import_env: BidsImportEnv, scans_file: BidsScansTsvFile, loris_scans_path: Path): +def add_bids_scan_row(import_env: BidsImportEnv, scan_row: BidsScanTsvRow, loris_scans_path: Path): """ - Copy some `scans.tsv` rows into a LORIS `scans.tsv` file, creating it if necessary. + Add a BIDS `scans.tsv` row into a LORIS `scans.tsv` file, creating it if necessary. """ - # Do not copy the file in no-copy mode. + # Do not copy the row in no-copy mode. if import_env.loris_bids_path is None: return scans_path = import_env.data_dir_path / loris_scans_path - if scans_path.exists(): - scans_file.merge(BidsScansTsvFile(scans_path)) - scans_path.parent.mkdir(parents=True, exist_ok=True) - scans_file.write(scans_path) + # Create the LORIS scans.tsv file if it does not exist yet. + scans_path.touch(exist_ok=True) + + scans_file = BidsScansTsvFile(scans_path) + scans_file.set_row(scan_row) + scans_file.write() diff --git a/python/loris_bids_importer/src/loris_bids_importer/eeg/main.py b/python/loris_bids_importer/src/loris_bids_importer/eeg/main.py index 0ea7714fe..28de8ca1f 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/eeg/main.py +++ b/python/loris_bids_importer/src/loris_bids_importer/eeg/main.py @@ -2,10 +2,8 @@ import json import os -import sys from pathlib import Path -import lib.exitcode import lib.utilities as utilities from lib.config import get_ephys_visualization_enabled_config from lib.db.models.physio_file import DbPhysioFile @@ -28,7 +26,12 @@ from loris_bids_importer.archive import import_physio_event_archive, import_physio_file_archive from loris_bids_importer.channels import insert_bids_channels_file -from loris_bids_importer.copy_files import copy_loris_bids_file, get_loris_bids_file_path, get_loris_scans_path +from loris_bids_importer.copy_files import ( + add_bids_scan_row, + copy_loris_bids_file, + get_loris_bids_file_path, + get_loris_scans_path, +) from loris_bids_importer.eeg.physiological import Physiological from loris_bids_importer.env import BidsImportEnv from loris_bids_importer.events import insert_bids_event_dict_file, insert_bids_events_file @@ -38,6 +41,7 @@ get_check_bids_physio_modality, get_check_bids_physio_output_type, ) +from loris_bids_importer.scans import add_bids_scans_file_parameters class Eeg: @@ -304,19 +308,10 @@ def fetch_and_insert_eeg_files(self, derivatives=False, detect=True): # get the acquisition date of the EEG file or the age at the time of the EEG recording eeg_acq_time = None if self.scans_file is not None: - scan_info = self.scans_file.get_row(eeg_file_path) - if scan_info is not None: - try: - eeg_acq_time = scan_info.get_acquisition_time() - eeg_file_data['age_at_scan'] = scan_info.get_age_at_scan() - except Exception as error: - print(f"ERROR: {error}") - sys.exit(lib.exitcode.PROGRAM_EXECUTION_FAILURE) - - loris_scans_path = get_loris_scans_path(self.info, self.scans_file, self.session) - eeg_file_data['scans_tsv_file'] = loris_scans_path - scans_blake2 = compute_file_blake2b_hash(self.scans_file.path) - eeg_file_data['physiological_scans_tsv_file_bake2hash'] = scans_blake2 + scan_row = self.scans_file.get_row(eeg_file_path) + if scan_row is not None: + eeg_acq_time = scan_row.get_acquisition_time() + add_bids_scans_file_parameters(self.info, self.session, self.scans_file, scan_row, eeg_file_data) # if file type is set and fdt file exists, append fdt path to the # eeg_file_data dictionary @@ -361,6 +356,15 @@ def fetch_and_insert_eeg_files(self, derivatives=False, detect=True): insert_physio_file_parameters(self.env, physio_file, eeg_file_data) self.env.db.commit() + # Update the LORIS scans.tsv file. + + if self.scans_file is not None: + scan_row = self.scans_file.get_row(eeg_file_path) + if scan_row is not None: + loris_scans_path = get_loris_scans_path(self.info, self.scans_file, self.session) + scan_row.set_file_name(eeg_path.name) + add_bids_scan_row(self.info, scan_row, loris_scans_path) + if self.info.loris_bids_path: # If we copy the file in assembly_bids and # if the EEG file was a set file, then update the filename for the .set diff --git a/python/loris_bids_importer/src/loris_bids_importer/main.py b/python/loris_bids_importer/src/loris_bids_importer/main.py index 538011ac2..acf6dbafb 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/main.py +++ b/python/loris_bids_importer/src/loris_bids_importer/main.py @@ -13,11 +13,9 @@ from loris_bids_importer.args import Args from loris_bids_importer.copy_files import ( copy_bids_participants_file, - copy_bids_scans_file, copy_bids_static_files, get_loris_bids_dataset_path, get_loris_bids_root_file_path, - get_loris_scans_path, ) from loris_bids_importer.eeg.main import Eeg from loris_bids_importer.env import BidsImportEnv @@ -152,17 +150,6 @@ def import_bids_session( # This should not happen as BIDS session labels should have been checked previously. log_error_exit(env, f"Visit not found for visit label '{visit_label}'.") - try: - # Read the scans.tsv property to raise an exception if the file is incorrect. - if bids_session.scans_file is not None: - loris_scans_path = get_loris_scans_path(import_env, bids_session.scans_file, session) - copy_bids_scans_file(import_env, bids_session.scans_file, loris_scans_path) - except Exception as exception: - log_warning( - env, - f"Error while reading the session scans.tsv file, scans.tsv data will be ignored. Full error:\n{exception}" - ) - # Process each data type directory. for data_type in bids_session.data_types: diff --git a/python/loris_bids_importer/src/loris_bids_importer/mri/main.py b/python/loris_bids_importer/src/loris_bids_importer/mri/main.py index 829b6988a..5915dbab2 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/mri/main.py +++ b/python/loris_bids_importer/src/loris_bids_importer/mri/main.py @@ -11,14 +11,13 @@ from lib.imaging_lib.nifti import add_nifti_spatial_file_parameters from lib.imaging_lib.nifti_pic import create_nifti_preview_picture from lib.imaging_lib.scan_type import create_mri_scan_type -from lib.logging import log from loris_bids_utils.info import BidsAcquisitionInfo from loris_bids_utils.mri.acquisition import MriAcquisition from loris_bids_utils.mri.reader import BidsMriDataTypeReader from loris_utils.crypto import compute_file_blake2b_hash from loris_utils.error import group_errors_tuple -from loris_bids_importer.acquisitions import import_bids_acquisitions +from loris_bids_importer.acquisitions import BidsImportFileResult, BidsImportFileStatus, import_bids_acquisitions from loris_bids_importer.copy_files import copy_loris_bids_file, get_loris_bids_file_path from loris_bids_importer.env import BidsImportEnv from loris_bids_importer.file_type import get_check_bids_imaging_file_type_from_extension @@ -55,6 +54,7 @@ def import_bids_mri_data_type( import_bids_acquisitions( env, import_env, + session, data_type.acquisitions, lambda acquisition, bids_info: import_bids_mri_acquisition( env, @@ -72,7 +72,7 @@ def import_bids_mri_acquisition( session: DbSession, acquisition: MriAcquisition, bids_info: BidsAcquisitionInfo, -): +) -> BidsImportFileResult: """ Import a BIDS NIfTI file and its associated files in LORIS. """ @@ -87,9 +87,7 @@ def import_bids_mri_acquisition( loris_file = try_get_file_with_path(env.db, loris_file_path) if loris_file is not None: - import_env.ignored_acquisitions_count += 1 - log(env, f"File '{loris_file_path}' is already registered in LORIS. Skipping.") - return + return BidsImportFileResult(BidsImportFileStatus.IGNORE, loris_file_path) # Get information about the file. @@ -139,7 +137,7 @@ def import_bids_mri_acquisition( file_parameters['file_blake2b_hash'] = file_hash if bids_info.scans_file is not None and bids_info.scan_row is not None: - add_bids_scans_file_parameters(bids_info.scans_file, bids_info.scan_row, file_parameters) + add_bids_scans_file_parameters(import_env, session, bids_info.scans_file, bids_info.scan_row, file_parameters) for aux_file_type, aux_file_path in aux_file_paths: aux_file_hash = compute_file_blake2b_hash(aux_file_path) @@ -178,6 +176,8 @@ def import_bids_mri_acquisition( create_nifti_preview_picture(env, file) + return BidsImportFileResult(BidsImportFileStatus.SUCCESS, loris_file_path) + def get_check_bids_nifti_file_hash(env: Env, acquisition: MriAcquisition) -> str: """ diff --git a/python/loris_bids_importer/src/loris_bids_importer/scans.py b/python/loris_bids_importer/src/loris_bids_importer/scans.py index 2523560d6..fc39fca0d 100644 --- a/python/loris_bids_importer/src/loris_bids_importer/scans.py +++ b/python/loris_bids_importer/src/loris_bids_importer/scans.py @@ -1,10 +1,16 @@ from typing import Any +from lib.db.models.session import DbSession from loris_bids_utils.files.scans import BidsScansTsvFile, BidsScanTsvRow from loris_utils.crypto import compute_file_blake2b_hash +from loris_bids_importer.copy_files import get_loris_scans_path +from loris_bids_importer.env import BidsImportEnv + def add_bids_scans_file_parameters( + import_env: BidsImportEnv, + session: DbSession, scans_file: BidsScansTsvFile, scan_row: BidsScanTsvRow, file_parameters: dict[str, Any], @@ -14,7 +20,7 @@ def add_bids_scans_file_parameters( dictionary. """ - file_parameters['scan_acquisition_time'] = scan_row.get_acquisition_time() - file_parameters['age_at_scan'] = scan_row.get_age_at_scan() - file_parameters['scans_tsv_file'] = scans_file.path - file_parameters['scans_tsv_file_bake2hash'] = compute_file_blake2b_hash(scans_file.path) + file_parameters['scan_acquisition_time'] = scan_row.get_acquisition_time() + file_parameters['age_at_scan'] = scan_row.get_age_at_scan() + file_parameters['scans_tsv_file'] = get_loris_scans_path(import_env, scans_file, session) + file_parameters['scans_tsv_file_blake2hash'] = compute_file_blake2b_hash(scans_file.path) diff --git a/python/loris_bids_utils/src/loris_bids_utils/files/scans.py b/python/loris_bids_utils/src/loris_bids_utils/files/scans.py index e26e4b652..1be544fe1 100644 --- a/python/loris_bids_utils/src/loris_bids_utils/files/scans.py +++ b/python/loris_bids_utils/src/loris_bids_utils/files/scans.py @@ -14,6 +14,27 @@ class BidsScanTsvRow(BidsTsvRow): Documentation: https://bids-specification.readthedocs.io/en/stable/modality-agnostic-files/data-summary-files.html#scans-file """ + file_path: Path + """ + The path of the scan file relative to the scans.tsv file. + """ + + def __init__(self, data: dict[str, str | None]): + super().__init__(data) + file_path = self.data.get('filename') + if file_path is None: + raise Exception("Missing filename field in `scans.tsv` file.") + + self.file_path = Path(file_path) + + def set_file_name(self, file_name: str): + """ + Set the name of the scan file of this row. + """ + + self.file_path = self.file_path.with_name(file_name) + self.data['filename'] = str(self.file_path) + def get_acquisition_time(self) -> datetime | None: """ Get the acquisition time of the acquisition file. @@ -63,14 +84,17 @@ def get_row(self, file_path: Path) -> BidsScanTsvRow | None: Get the row corresponding to the given file path. """ - return find(self.rows, lambda row: file_path.name == row.data['filename']) + # According to the specification, the 'filename' column is the path of the acquisition file + # relative to the directory in which the scans.tsv file is located. + relative_path = file_path.relative_to(self.path.parent) + return find(self.rows, lambda row: relative_path == row.file_path) def set_row(self, scan: BidsScanTsvRow): """ Add a row in the `scans.tsv` file, replacing it if a row already exists for its file name. """ - replace_or_append(self.rows, scan, lambda row: row.data['filename'] == scan.data['filename']) + replace_or_append(self.rows, scan, lambda row: row.file_path == scan.file_path) def merge(self, other: 'BidsScansTsvFile'): """ diff --git a/python/loris_bids_utils/src/loris_bids_utils/mri/reader.py b/python/loris_bids_utils/src/loris_bids_utils/mri/reader.py index cf9e77639..715ecc5a6 100644 --- a/python/loris_bids_utils/src/loris_bids_utils/mri/reader.py +++ b/python/loris_bids_utils/src/loris_bids_utils/mri/reader.py @@ -1,10 +1,9 @@ from dataclasses import dataclass from functools import cached_property -from pathlib import Path from bids.layout import BIDSFile -from loris_utils.path import remove_path_extension +from loris_utils.path import remove_path_extension, replace_path_extension from loris_bids_utils.info import BidsAcquisitionInfo from loris_bids_utils.mri.acquisition import MriAcquisition @@ -29,17 +28,20 @@ def acquisitions(self) -> list[tuple[MriAcquisition, BidsAcquisitionInfo]]: for pybids_file in pybids_files: nifti_path = get_pybids_file_path(pybids_file) - # Get all associated files - associations: list[BIDSFile] = pybids_file.get_associations() # type: ignore + sidecar_path = replace_path_extension(nifti_path, 'json') + if not sidecar_path.exists(): + sidecar_path = None - # Find associated files using predicates - sidecar_path = find_pybids_file_path(associations, lambda file: file.entities.get('extension') == '.json') + bval_path = replace_path_extension(nifti_path, 'bval') + if not bval_path.exists(): + bval_path = None - pybids_bval_path = pybids_layout.get_nearest(pybids_file, extension='.bval') # type: ignore - bval_path = Path(pybids_bval_path) if pybids_bval_path is not None else None # type: ignore + bvec_path = replace_path_extension(nifti_path, 'bvec') + if not bvec_path.exists(): + bvec_path = None - pybids_bvec_path = pybids_layout.get_nearest(pybids_file, extension='.bvec') # type: ignore - bvec_path = Path(pybids_bvec_path) if pybids_bvec_path is not None else None # type: ignore + # Get all associated files + associations: list[BIDSFile] = pybids_file.get_associations() # type: ignore events_path = find_pybids_file_path( associations, diff --git a/python/loris_bids_utils/src/loris_bids_utils/tsv.py b/python/loris_bids_utils/src/loris_bids_utils/tsv.py index c38cf1e2b..a18618e0a 100644 --- a/python/loris_bids_utils/src/loris_bids_utils/tsv.py +++ b/python/loris_bids_utils/src/loris_bids_utils/tsv.py @@ -58,11 +58,14 @@ def get_field_names(self) -> list[str]: return fields - def write(self, path: Path): + def write(self, path: Path | None = None): """ - Write the TSV file to a file at the given path, creating it if necessary. + Write the TSV file at the given path. If the path is not provided, the path of this file is + used, overwriting existing data. """ + path = path if path is not None else self.path + fields = self.get_field_names() with open(path, 'w', newline='') as file: diff --git a/python/loris_utils/src/loris_utils/fs.py b/python/loris_utils/src/loris_utils/fs.py index 267fe34c1..7fa14c58a 100644 --- a/python/loris_utils/src/loris_utils/fs.py +++ b/python/loris_utils/src/loris_utils/fs.py @@ -29,7 +29,7 @@ def iter_all_dir_files(dir_path: Path) -> Iterator[Path]: for file_path in dir_path.rglob('*'): if file_path.is_file(): - yield file_path + yield file_path.relative_to(dir_path) def is_directory_empty(dir_path: Path) -> bool: diff --git a/python/tests/integration/scripts/test_import_bids_dataset.py b/python/tests/integration/scripts/test_import_bids_dataset.py index 15fcb9fe7..f460a53c4 100644 --- a/python/tests/integration/scripts/test_import_bids_dataset.py +++ b/python/tests/integration/scripts/test_import_bids_dataset.py @@ -1,3 +1,4 @@ +from datetime import datetime from pathlib import Path from lib.db.queries.bids_event_dataset_mapping import get_bids_event_dataset_mappings_with_project_id @@ -27,9 +28,7 @@ def test_import_eeg_bids_dataset(): # Check the return code. assert process.returncode == 0 - assert process.stderr == ( - "WARNING: No 'scans.tsv' file found, 'scans.tsv' data will be ignored.\n" - ) + assert process.stderr == "" # Check that the candidate and sessions are present in the database. candidate = try_get_candidate_with_psc_id(db, 'OTT166') @@ -47,16 +46,17 @@ def test_import_eeg_bids_dataset(): assert len(file.channels) == 128 assert len(file.event_files) == 1 assert len(file.task_events) == 3185 + assert file.acquisition_time == datetime(2025, 10, 10, 15, 1, 10) assert file.archive is not None - assert file.archive.path == Path('bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_eeg.tgz') # noqa: E501 + assert file.archive.path == Path('bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_eeg.tgz') # ruff:ignore[line-too-long] assert file.event_archive is not None - assert file.event_archive.path == Path('bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_events.tgz') # noqa: E501 + assert file.event_archive.path == Path('bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_events.tgz') # ruff:ignore[line-too-long] # Check that the physiological file parameters has been inserted in the database. file_parameters = get_physio_file_parameters_dict(db, file.id) assert file_parameters == { 'TaskName': 'FaceHouseCheck', - 'TaskDescription': 'Visual presentation of oval cropped face and house images both upright and inverted. Rare left or right half oval checkerboards were presetned as targets for keypress response.', # noqa: E501 + 'TaskDescription': 'Visual presentation of oval cropped face and house images both upright and inverted. Rare left or right half oval checkerboards were presetned as targets for keypress response.', # ruff:ignore[line-too-long] 'InstitutionName': 'Brock University', 'InstitutionAddress': '500 Glenridge Ave, St.Catharines, Ontario', 'SamplingFrequency': '256', @@ -68,20 +68,24 @@ def test_import_eeg_bids_dataset(): 'MiscChannelCount': '0', 'TriggerChannelCount': '0', 'PowerLineFrequency': '60', - 'EEGPlacementScheme': 'Custom equidistant 128 channel BioSemi montage established in coordination with Judith Schedden McMaster Univertisy', # noqa: E501 + 'EEGPlacementScheme': 'Custom equidistant 128 channel BioSemi montage established in coordination with Judith Schedden McMaster Univertisy', # ruff:ignore[line-too-long] 'Manufacturer': 'BioSemi', 'CapManufacturer': 'ElectroCap International', 'HardwareFilters': 'n/a', 'SoftwareFilters': 'n/a', 'RecordingType': 'continuous', 'RecordingDuration': '1119', - 'eegjson_file': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_eeg.json', # noqa: E501 - 'physiological_json_file_blake2b_hash': 'f762bbf2e4699fbe47a53f2b7c2f990dc401d7aa57a6b4ba37aa04acdc2748feb42ee058b874a14fae7c14f673280847e40a60771b3c397a0cf5abdb8c05077a', # noqa: E501 - 'physiological_file_blake2b_hash': '8c24c5907b724d659f38c65bfffc754003a586961ea9e25ed5fa0741a2691ed217e29553020230553c84c0b5978edf64624747d31623f6267cbd36eba8b70891', # noqa: E501 - 'electrode_file_blake2b_hash': '0206db2650ae5a07e4225ff87b6fb2a6bfcf6ea088dd8cdfd77d04e9e25a171ffe68878aa156478babb32dcaf9b46459f87fe0516b728278cc0d9372a0d49299', # noqa: E501 - 'channel_file_blake2b_hash': '7b91e3650086ef50ecc00f1c50e17e7ad8dc39c484536bbc2423af4be7d2b50a3a0010f840d457fec68fbfb3e136edf4d616a31bab0ca09ed686f555727341dd', # noqa: E501 - 'event_file_blake2b_hash': '532aa0b52749eb9ee52c2bbb65fa7b1d00d7126cb9a4e10bd4b9dbb4c5527b06e30acdaf17d5806e81d3ce8ad224a9f456e27aba1bf8b92fd43522837c7ffec7', # noqa: E501 - 'electrophysiology_chunked_dataset_path': 'chunks/Face13_BIDSVersion_1.1.0_chunks/sub-OTT166_ses-V1_task-faceO_eeg.chunks', # noqa: E501 + 'eegjson_file': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/eeg/sub-OTT166_ses-V1_task-faceO_eeg.json', # ruff:ignore[line-too-long] + 'physiological_json_file_blake2b_hash': 'f762bbf2e4699fbe47a53f2b7c2f990dc401d7aa57a6b4ba37aa04acdc2748feb42ee058b874a14fae7c14f673280847e40a60771b3c397a0cf5abdb8c05077a', # ruff:ignore[line-too-long] + 'physiological_file_blake2b_hash': '8c24c5907b724d659f38c65bfffc754003a586961ea9e25ed5fa0741a2691ed217e29553020230553c84c0b5978edf64624747d31623f6267cbd36eba8b70891', # ruff:ignore[line-too-long] + 'electrode_file_blake2b_hash': '0206db2650ae5a07e4225ff87b6fb2a6bfcf6ea088dd8cdfd77d04e9e25a171ffe68878aa156478babb32dcaf9b46459f87fe0516b728278cc0d9372a0d49299', # ruff:ignore[line-too-long] + 'channel_file_blake2b_hash': '7b91e3650086ef50ecc00f1c50e17e7ad8dc39c484536bbc2423af4be7d2b50a3a0010f840d457fec68fbfb3e136edf4d616a31bab0ca09ed686f555727341dd', # ruff:ignore[line-too-long] + 'event_file_blake2b_hash': '532aa0b52749eb9ee52c2bbb65fa7b1d00d7126cb9a4e10bd4b9dbb4c5527b06e30acdaf17d5806e81d3ce8ad224a9f456e27aba1bf8b92fd43522837c7ffec7', # ruff:ignore[line-too-long] + 'electrophysiology_chunked_dataset_path': 'chunks/Face13_BIDSVersion_1.1.0_chunks/sub-OTT166_ses-V1_task-faceO_eeg.chunks', # ruff:ignore[line-too-long] + 'scan_acquisition_time': '2025-10-10 15:01:10.720100+00:00', + 'age_at_scan': 'None', + 'scans_tsv_file': 'bids_imports/Face13_BIDSVersion_1.1.0/sub-OTT166/ses-V1/sub-OTT166_ses-V1_scans.tsv', + 'scans_tsv_file_blake2hash': '4d9b695ba6b35257531b96375ca15c36179b08360f373c46425d958e406132b84ad029113ed4be5e458e762a8af0792ddde5127b5044778c8c8705d8df8f8621', # ruff:ignore[line-too-long] } # Check that the event files has been inserted in the database. @@ -97,6 +101,7 @@ def test_import_eeg_bids_dataset(): 'README': None, 'sub-OTT166': { 'ses-V1': { + 'sub-OTT166_ses-V1_scans.tsv': None, 'eeg': { 'sub-OTT166_ses-V1_task-faceO_channels.tsv': None, 'sub-OTT166_ses-V1_task-faceO_eeg.edf': None, diff --git a/python/tests/integration/scripts/test_mass_nifti_pic.py b/python/tests/integration/scripts/test_mass_nifti_pic.py index a08ed9eb1..8221e65e0 100644 --- a/python/tests/integration/scripts/test_mass_nifti_pic.py +++ b/python/tests/integration/scripts/test_mass_nifti_pic.py @@ -25,7 +25,7 @@ def test_invalid_profile_arg(): # Check return code, STDOUT and STDERR assert process.returncode == INVALID_PATH assert process.stdout == "" - assert process.stderr == "ERROR: No configuration file 'invalid_profile.py' found in the '/opt/loris/bin/mri/config' directory.\n" # noqa: E501 + assert process.stderr == "ERROR: No configuration file 'invalid_profile.py' found in the '/opt/loris/bin/mri/config' directory.\n" # ruff:ignore[line-too-long] def test_smallest_id_bigger_than_largest_id(): @@ -76,7 +76,7 @@ def test_on_file_id_that_already_has_a_pic(): # Check return code, STDOUT and STDERR assert process.returncode == SUCCESS assert process.stdout == "" - assert process.stderr == "WARNING: There is already a pic for file ID 2. Use -f or --force to overwrite it, skipping.\n" # noqa: E501 + assert process.stderr == "WARNING: There is already a pic for file ID 2. Use -f or --force to overwrite it, skipping.\n" # ruff:ignore[line-too-long] def test_force_option(): diff --git a/tools/delete_imaging_upload.pl b/tools/delete_imaging_upload.pl index d0d75a6bd..f5ab58c72 100755 --- a/tools/delete_imaging_upload.pl +++ b/tools/delete_imaging_upload.pl @@ -1802,8 +1802,6 @@ sub deleteUploadsInDatabase { $nbRecordsDeleted += &deleteTableData($dbh, 'tarchive', 'TarchiveID', [$tarchiveID], $tmpSQLFile, $optionsRef) if defined $tarchiveID; } - &updateSessionTable($dbh, $filesRef->{'mri_upload'}, $tmpSQLFile) unless @$scanTypesToDeleteRef || $optionsRef->{'BASENAME'} ne ''; - $dbh->commit; # If the SQL restore file should be produced @@ -1856,54 +1854,6 @@ sub gzipBackupFile { =pod -=head3 updateSessionTable($dbh, $mriUploadsRef, $tmpSQLFile) - -Sets to C the C column of all C in the database that do not have an associated upload -after the script has deleted those whose IDs are passed on the command line. The script also adds an SQL statement -in the SQL file whose path is passed as argument to restore the state that the C table had before the deletions. - -INPUTS: - - $dbh : database handle. - - $mriUploadsRef: reference on an array of hashes containing the uploads to delete. Accessed like this: - C<< $mriUploadsRef->[0]->{'TarchiveID'} >>(this would return the C of the first C - in the array. The properties stored for each hash are: C, C, C - C, C and C. - - $tmpSQLFile: path of the SQL file that contains the SQL statements used to restore the deleted records. - -=cut -sub updateSessionTable { - my($dbh, $mriUploadsRef, $tmpSQLFile) = @_; - - # If any of the uploads to delete is the last upload that was part of the - # session associated to it, then set the session's 'Scan_done' flag - # to 'N'. - my @sessionIDs = map { $_->{'SessionID'} } @$mriUploadsRef; - @sessionIDs = grep(defined $_, @sessionIDs); - - return if !@sessionIDs; - - my $query = "UPDATE session s SET Scan_done = 'N'" - . " WHERE s.ID IN (" - . join(',', ('?') x @sessionIDs) - . ") AND (SELECT COUNT(*) FROM mri_upload m WHERE m.SessionID=s.ID) = 0"; - $dbh->do($query, undef, @sessionIDs ); - - if($tmpSQLFile) { - # Write an SQL statement to restore the 'Scan_done' column of the deleted uploads - # to their appropriate values. This statement needs to be after the statement that - # restores table mri_upload (at the end of the file is good enough). - open(SQL, ">>$tmpSQLFile") or die "Cannot append text to file $tmpSQLFile: $!. Aborting.\n"; - print SQL "\n\n"; - print SQL "UPDATE session s SET Scan_done = 'Y'" - . " WHERE s.ID IN (" - . join(',', @sessionIDs) - . ") AND (SELECT COUNT(*) FROM mri_upload m WHERE m.SessionID=s.ID) > 0;\n"; - close(SQL); - } -} - -=pod - =head3 updateFilesIntermediaryTable($dbh, $filesRef, $tmpSQLFile) Sets the C and C columns of all the defaced files to C<$tarchiveID> and C diff --git a/uploadNeuroDB/NeuroDB/MRI.pm b/uploadNeuroDB/NeuroDB/MRI.pm index acd666ca0..a76fa4a12 100755 --- a/uploadNeuroDB/NeuroDB/MRI.pm +++ b/uploadNeuroDB/NeuroDB/MRI.pm @@ -1084,19 +1084,17 @@ sub createNewCandID { =head3 getPSC($patientName, $dbhr, $db) -Looks for the site alias using the C table C as -a first resource, for the cases where it is created using the front-end, -otherwise, find the site alias in whatever field (usually C -or C) is provided, and return the C and C. +Looks for the C of a scan. If the configuration already provides a +C, it is validated against the C table. Otherwise, the C +is looked up using the C table, and as a last resort by matching the site alias against whatever field +(usually C or C) is provided. INPUTS: - $patientName: patient name - $dbhr : database handle reference - $db : database object -RETURNS: a two element array: - - first is the MRI alias of the PSC or "UNKN" - - second is the C or 0 +RETURNS: the C or 0 if no center could be found =cut @@ -1110,14 +1108,26 @@ sub getPSC { $dbhr, $db ); + ## If the configuration (e.g. the prod file) already provides a CenterID, + ## validate it against the psc table rather than trusting it blindly or + ## falling through to the error-prone patient-name matching below. + if ($subjectIDsref->{'CenterID'}) { + my $centerID = $subjectIDsref->{'CenterID'}; + my $pscOB = NeuroDB::objectBroker::PSCOB->new( db => $db ); + my $pscsRef = $pscOB->get({ CenterID => $centerID }); + if (@$pscsRef) { + return $centerID; + } + return 0; + } + my $PSCID = $subjectIDsref->{'PSCID'}; my $visitLabel = $subjectIDsref->{'visitLabel'}; ## Get the CenterID from the session table, if the PSCID and visit labels exist ## and could be extracted if ($PSCID && $visitLabel) { - my $query = "SELECT s.CenterID, p.MRI_alias FROM session s - JOIN psc p on p.CenterID=s.CenterID + my $query = "SELECT s.CenterID FROM session s JOIN candidate c on c.ID=s.CandidateID WHERE c.PSCID = ? AND s.Visit_label = ?"; @@ -1125,21 +1135,21 @@ sub getPSC { $sth->execute($PSCID, $visitLabel); if ($sth->rows > 0) { my $row = $sth->fetchrow_hashref(); - return ($row->{'MRI_alias'},$row->{'CenterID'}); + return $row->{'CenterID'}; } } - ## Otherwise, use the patient name to match it to the site alias or MRI alias + ## Otherwise, use the patient name to match it to the site alias my $pscOB = NeuroDB::objectBroker::PSCOB->new( db => $db ); - my $pscsRef = $pscOB->get({ MRI_alias => { NOT => '' } }); + my $pscsRef = $pscOB->get({ Alias => { NOT => '' } }); foreach my $psc (@$pscsRef) { - if ($patientName =~ /$psc->{'Alias'}/i || $patientName =~ /$psc->{'MRI_alias'}/i) { - return ($psc->{'MRI_alias'}, $psc->{'CenterID'}); + if ($patientName =~ /$psc->{'Alias'}/i) { + return $psc->{'CenterID'}; } } - return ("UNKN", 0); + return 0; } =pod diff --git a/uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm b/uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm index c54ba72eb..5225dc11b 100755 --- a/uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm +++ b/uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm @@ -2311,11 +2311,12 @@ sub getUploadIDUsingTarchiveSrcLoc { =head3 getCenterNameFromCenterID($centerID) -Gets the MRI ALIAS form the C table using the center ID. +Gets the human-readable center C from the C table using the center ID. +Used to display a readable center name (rather than a numeric ID) in the logs. INPUT: Center ID -RETURNS: MRI ALIAS +RETURNS: center Name =cut @@ -2325,17 +2326,17 @@ sub getCenterNameFromCenterID { my $centerID = shift; my $query = ''; - my $alias = undef; + my $name = undef; if ($centerID) { - $query = "SELECT MRI_alias FROM psc WHERE CenterID =?"; + $query = "SELECT Name FROM psc WHERE CenterID =?"; my $sth = $dbh->prepare($query); $sth->execute($centerID); if ( $sth->rows > 0 ) { - $alias = $sth->fetchrow_array; + $name = $sth->fetchrow_array; } } - return $alias[0]; + return $name; } diff --git a/uploadNeuroDB/NeuroDB/objectBroker/PSCOB.pm b/uploadNeuroDB/NeuroDB/objectBroker/PSCOB.pm index 06010789d..31ea9abee 100644 --- a/uploadNeuroDB/NeuroDB/objectBroker/PSCOB.pm +++ b/uploadNeuroDB/NeuroDB/objectBroker/PSCOB.pm @@ -43,7 +43,7 @@ NeuroDB::objectBroker::PSCOB -- An object broker for records stored in table C

get( - { MRI_alias => 'my_alias' } + { Alias => 'my_alias' } ); foreach (@$pscRef) { printf "ID for PSC named $_->{'Name'} is $_->{'ID'}\n"; @@ -86,7 +86,7 @@ use TryCatch; my $TABLE_NAME = "psc"; -my @COLUMN_NAMES = qw(CenterID Name Alias MRI_alias); +my @COLUMN_NAMES = qw(CenterID Name Alias); =pod diff --git a/uploadNeuroDB/tarchiveLoader.pl b/uploadNeuroDB/tarchiveLoader.pl index 0bb32fda3..dcff45f3b 100755 --- a/uploadNeuroDB/tarchiveLoader.pl +++ b/uploadNeuroDB/tarchiveLoader.pl @@ -377,7 +377,6 @@ =head2 Methods ################################################################ my $centerID = $utility->determinePSC(\%tarchiveInfo, 0, $upload_id); -my $mri_alias = $utility->getCenterNameFromCenterID($centerID); ################################################################ ######### Determine the ScannerID ############################## @@ -603,10 +602,15 @@ =head2 Methods $notify_notsummary); } +################################################################ +#################### Get the center name ####################### +################################################################ +my $center_name = $utility->getCenterNameFromCenterID($centerID); + ################################################################ # make final logfile name without overwriting phantom logs ##### ################################################################ -my $final_logfile = $mri_alias; +my $final_logfile = $center_name; unless ($tarchiveInfo{'DateAcquired'} && $subjectIDsref->{'CandID'}) { ### if something went wrong and there is no acq date or CandID $final_logfile .= '_'.$temp[$#temp];