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.

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

MIRIAD converter to BIDS #1290

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

MatthieuJoulot
Copy link
Contributor

Implementation of a BIDS converter for the MIRIAD cohort.

@MatthieuJoulot
Copy link
Contributor Author

MatthieuJoulot commented Sep 9, 2024

Status right now:

  • Very quick and very dirty (ofc not tested, ofc not documented)
  • converts the database
  • Does not take into account any clinical data (even though it takes a clinical_data directory in input)
  • does not pass tests

Copy link
Contributor

@AliceJoubert AliceJoubert left a comment

Choose a reason for hiding this comment

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

Thanks for the work @MatthieuJoulot ! I made a few... lots of comments actually ^^'. Tell me if you need clarification.

return value
raise ValueError(
f"BIDS MIRIAD subject ID {value} is not properly formatted. "
"Expecting a 'sub-MIRIAD' format."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Expecting a 'sub-MIRIAD' format."
"Expecting a 'sub-MIRIADXXX' format."

@@ -319,6 +321,29 @@ def from_original_study_id(cls, study_id: str) -> str:
def to_original_study_id(self) -> str:
return str(self.replace("sub-", ""))

class MIRIADBIDSSubjectID(BIDSSubjectID):
"""Implementation for MIRIAD of the BIDSSubjectIDClass, allowing to go from the source id MIRIAD###
to a bids id sub-MIRAD### and reciprocally."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
to a bids id sub-MIRAD### and reciprocally."""
to a bids id sub-MIRIAD### and reciprocally."""

return f"sub-{study_id}"
raise ValueError(
f"Raw MIRIAD subject ID {study_id} is not properly formatted. "
"Expecting a 'Y' format."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Expecting a 'Y' format."
"Expecting a 'MIRIADXXX' format."

Comment on lines 12 to 14
input_dir = 'your_input_directory' # Where the original data is located
output_dir = 'your_output_directory' # Where the BIDS data will be written
csv_file = 'metadata.csv' # Metadata CSV file to store extracted information
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
input_dir = 'your_input_directory' # Where the original data is located
output_dir = 'your_output_directory' # Where the BIDS data will be written
csv_file = 'metadata.csv' # Metadata CSV file to store extracted information

You can directly use the paths given to the convert function :

  • input_dir < > path_to_dataset
  • output_dir < > bids_dir

And if you use 'metadata.csv' only once I would not define a variable specifically for this, you can use it as is inside convert

subjects (Optional[UserProvidedPath], optional): _description_. Defaults to None.
n_procs (Optional[int], optional): _description_. Defaults to 1.
"""
from clinica.iotools.converters.miriad_to_bids.miriad_to_bids_utils import create_bids_structure
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from clinica.iotools.converters.miriad_to_bids.miriad_to_bids_utils import create_bids_structure
from clinica.iotools.converters.miriad_to_bids.miriad_to_bids_utils import create_bids_structure
from ..utils import validate_input_path
path_to_dataset = validate_input_path(path_to_dataset)
bids_dir = validate_input_path(bids_dir, check_exist=False)
if n_procs != 1:
cprint(
f"{get_converter_name(StudyName.MIRIAD)} converter does not support multiprocessing yet. n_procs set to 1.",
lvl="warning",
)
if not subjects:
#todo

First step there would be to check the inputs of the convert function :

  • your paths : the function given here verify existence go the path if needed and return a Path object, so we are sure we get always the same type of objects when it comes to user-given paths.
  • optional parameters : n_procs is not implemented (and I doubt you want to) so we just warn the user if she/he tries to use it. Should be the same for subjects 😁

Makes me wonder though, you do not plan on using any clinical data as an input ?

input_file = os.path.join(root, file)

# Create BIDS structure and move the file
create_bids_structure(subject_id, session, cohort, diagnosis, gender, input_file, path_to_dataset, bids_dir, path_to_clinical)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
create_bids_structure(subject_id, session, cohort, diagnosis, gender, input_file, path_to_dataset, bids_dir, path_to_clinical)
create_bids_structure(subject_id, session, file_path, bids_dir)

Comment on lines 60 to 61
bids_filename = f"sub-{subject_id}_ses-{session}_T1w.nii.gz"
output_file = os.path.join(f"sub-{subject_id}", f"ses-{session}", 'anat', bids_filename)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bids_filename = f"sub-{subject_id}_ses-{session}_T1w.nii.gz"
output_file = os.path.join(f"sub-{subject_id}", f"ses-{session}", 'anat', bids_filename)
output_file = f"sub-MIRIAD{subject_id}/ses-{session}/anat/sub-MIRIAD{subject_id}_ses-{session}_T1w.nii.gz"

parts = file.split('_')

# Extract information from filename
cohort = parts[0] # miriad
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cohort = parts[0] # miriad

If it is always the same as I assumed above in my regex you do not need to retrieve the info anymore

# Write the extracted information to CSV
bids_filename = f"sub-{subject_id}_ses-{session}_T1w.nii.gz"
output_file = os.path.join(f"sub-{subject_id}", f"ses-{session}", 'anat', bids_filename)
csvwriter.writerow([cohort, subject_id, diagnosis, gender, session, input_file, output_file])
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want the "subject_id" to be MIRIADXXX or XXX ?

)

@classmethod
def from_original_study_id(cls, study_id: str) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

As written you should have ids of 3 digits always (so there would need to be a padding if it can be 1 or 2). Though I don't think you are using that class for now

@NicolasGensollen NicolasGensollen added this to the v0.10.0 milestone Sep 10, 2024
@MatthieuJoulot
Copy link
Contributor Author

Alright clinical data have been added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants