aces / Loris-MRI

The set of scripts that preprocess and insert MRI data into the database.
10 stars 51 forks source link

Add file, file_parameter and scanner SQLAlchemy models #1207

Closed maximemulder closed 1 week ago

maximemulder commented 2 weeks ago

In order to improve the integration tests for run_dicom_archive_loader.py. I want to query the files table to make sure the files are inserted in the database. This PR adds SQLAlchemy model definitions for this table (and parameter_file and mri_scanner because why not).

The integration tests pass therefore the models are correct !

maximemulder commented 2 weeks ago

Just one quick question. I named the model for the files table mri_file.py. Is this name correct ? Or can there be files that are not MRI @cmadjar ?

cmadjar commented 2 weeks ago

@maximemulder there can be files in the files table that are not MRI. I would change to imaging_file.py instead so that any imaging modality is covered (PET, MRI, MRS etc..)

maximemulder commented 2 weeks ago

Ahok makes sense. Do you prefer just file.py or imaging_file.py ? The latter seems a little long to me but if you prefer it I'll use it instead. I could also use img_file.py but I don't think we use img as a shorthand of imaging anywhere else :thinking:

cmadjar commented 2 weeks ago

I think just file.py makes sense

maximemulder commented 2 weeks ago

Okay, I changed the name ! By the way I used file_parameter.py instead of parameter_file.py because it makes more sense to group the parameters of a file it with the files than with the parameter types to me. But this is a weak opinion, if you think parameter_file.py is a better name that's fine by me.

cmadjar commented 2 weeks ago

Probably best to stick with parameter_file like the table name to avoid confusions.

maximemulder commented 1 week ago

Updated ! Once this is merged I will update the integration test PR to also ensure the files have been inserted in the database (hopefully Friday).