From 46468149daffab9ec2b778466b86fd8a35815410 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 4 Oct 2023 15:55:13 -0400 Subject: [PATCH 01/10] [ENH] Add function to detect shared xnat subjects --- datman/xnat.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/datman/xnat.py b/datman/xnat.py index 18d9a81a..322efdd2 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -1507,6 +1507,22 @@ def assign_scan_names(self, config, ident): f"in session {str(ident)}. Reason {type(e).__name__}: " f"{e}") + def is_shared(self): + """Detect if the experiment is shared from another XNAT Project. + + Shared sessions have identical metadata to their source sessions, + the only way to tell a link apart from source data is to look for a + 'sharing/share' entry and check its xnat label. + """ + if self.subject in self.name: + return False + + share_entry = self._get_contents('sharing/share') + if not share_entry: + return False + + return self.subject in share_entry[0][0]['data_fields']['label'] + def __str__(self): return f"" From b642855c3bdb844403baab8bb4a619eeb2a66c2a Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 11 Oct 2023 18:30:06 -0400 Subject: [PATCH 02/10] [ENH] Add SharedExporter to handle shared XNAT sessions --- datman/exporters.py | 225 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 222 insertions(+), 3 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index fa78995b..8eed495e 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -19,10 +19,13 @@ import pydicom as dicom +import datman.config import datman.dashboard -from datman.exceptions import UndefinedSetting, DashboardException -from datman.scanid import (parse_bids_filename, ParseException, make_filename, - KCNIIdentifier) +import datman.scan +from datman.exceptions import (UndefinedSetting, DashboardException, + ExportException) +from datman.scanid import (parse, parse_bids_filename, ParseException, + make_filename, KCNIIdentifier) from datman.utils import (run, make_temp_directory, get_extension, filter_niftis, find_tech_notes, read_blacklist, get_relative_source, read_json, write_json) @@ -134,6 +137,8 @@ class SessionExporter(Exporter): def __init__(self, config, session, experiment, dry_run=False, **kwargs): self.experiment = experiment + self.config = config + self.session = session self.dry_run = dry_run def __repr__(self): @@ -969,6 +974,220 @@ def errors_outdated(self, scan, fname): return False +class SharedExporter(SessionExporter): + """Export an XNAT 'shared' experiment. + """ + + type = "shared" + ext = ".nii.gz" + + def __init__(self, config, session, experiment, bids_opts=None, **kwargs): + if not experiment.is_shared(): + raise ExportException( + f"Cannot make SharedExporter for {experiment}. " + "XNAT Experiment is not shared." + ) + + try: + self.source_session = self.find_source_session(config, experiment) + except (ParseException, datman.config.ConfigException) as exc: + raise ExportException( + f"Can't find source data for shared experiment {experiment}. " + f"Reason - {exc}" + ) + + # The datman-style directories to export + dm_dirs = ['qc_path', 'dcm_path', 'mnc_path', 'nrrd_path'] + if not bids_opts: + dm_dirs.append('nii_path') + + super().__init__(config, session, experiment, **kwargs) + + self.name_map = self.make_name_map(dm_dirs, use_bids=bids_opts) + + def find_source_session(self, config, experiment): + """Find the original data on the filesystem. + + Args: + config (:obj:`datman.config.config`): The datman config object for + the study that the shared experiment belongs to. + experiment (:obj:`datman.xnat.XNATExperiment`): The experiment + object for the shared session on XNAT. + + Returns: + :obj:`datman.scan.Scan`: The scan object for the source dataset + as previously exported to the filesystem. + """ + ident = parse(experiment.name) + study = config.map_xnat_archive_to_project(ident) + config = datman.config.config(study=study) + return datman.scan.Scan(ident, config) + + def make_name_map(self, dm_dirs, use_bids=False): + """Create a dictionary of source files to their 'shared' alias. + + Args: + dm_dirs (:obj:`list`): A list of datman-style paths on the source + session's :obj:`datman.scan.Scan` object to search for files. + use_bids (any, optional): Whether or not to search for bids files. + Any input equivalent to boolean True will be taken as 'True'. + Default False. + + Returns: + dict: A dictionary mapping the full path to each discovered source + file to the full path to the output alias name/symlink. + """ + if use_bids: + name_map = self.find_bids_files() + else: + name_map = {} + + for dir_type in dm_dirs: + self.find_dm_files(dir_type, name_map) + + self.find_resource_files(name_map) + + return name_map + + def find_bids_files(self, name_map=None): + """Find all bids files that have been created for the source session. + + Args: + name_map (dict, optional): A dictionary that may contain other + discovered files and their output names. Default None. + + Returns: + dict: A dictionary of source session files that have been + found, mapped to their full path under the shared/alias ID. + """ + if name_map is None: + name_map = {} + + for root, _, files in os.walk(self.source_session.bids_path): + dest_dir = root.replace( + self.source_session.bids_path, + self.session.bids_path + ) + + for item in files: + dest_name = item.replace( + self.source_session.bids_sub, self.session.bids_sub + ).replace( + self.source_session.bids_ses, self.session.bids_ses + ) + name_map[os.path.join(root, item)] = os.path.join( + dest_dir, dest_name + ) + + return name_map + + def find_dm_files(self, dir_type, name_map=None): + """Find datman-style source files in all listed directory types. + + Args: + dir_type (list): A list of paths on the source session to search + through. All entries should be valid path types defined for + the `datman.scan.Scan` object. + name_map (dict, optional): A dictionary of other discovered + source files mapped to their aliases. Default None. + + Returns: + dict: A dictionary of source session files that have been + found, mapped to their full path under the shared/alias ID. + """ + if name_map is None: + name_map = {} + + source_dir = getattr(self.source_session, dir_type) + dest_dir = getattr(self.session, dir_type) + + for scan in self.experiment.scans: + for name in scan.names: + if read_blacklist(scan=name, config=self.config): + continue + + source_name = name.replace( + self.session.id_plus_session, + self.source_session.id_plus_session + ) + for match in glob(os.path.join(source_dir, f"{source_name}*")): + ext = get_extension(match) + name_map[match] = os.path.join(dest_dir, name + ext) + + return name_map + + def find_resource_files(self, name_map=None): + """Find all source session resources files. + + Args: + name_map (dict, optional): A dictionary of any previously found + source files mapped to their aliases. + + Returns: + dict: A dictionary of source session files that have been + found, mapped to their full path under the shared/alias ID. + """ + if name_map is None: + name_map = {} + + for root, _, files in os.walk(self.session.resource_path): + dest_path = root.replace( + self.source_session.resource_path, + self.session.resource_path + ) + for item in files: + name_map[os.path.join(root, item)] = os.path.join( + dest_path, item + ) + + return name_map + + def export(self, *args, **kwargs): + if self.dry_run: + logger.info( + "Dry run: Skipping export of shared session files " + f"{self.name_map}" + ) + return + + for source in self.name_map: + parent, _ = os.path.split(self.name_map[source]) + try: + os.makedirs(parent) + except FileExistsError: + pass + except PermissionError: + logger.error( + f"Failed to make dir {parent} for session {self.session}. " + "Permission denied." + ) + continue + rel_source = get_relative_source(source, self.name_map[source]) + try: + os.symlink(rel_source, self.name_map[source]) + except FileExistsError: + pass + except PermissionError: + logger.error( + f"Failed to create {self.name_map[source]}. " + "Permission denied." + ) + + def outputs_exist(self): + for source in self.name_map: + if not os.path.islink(self.name_map[source]): + # Check if there's a link, NOT whether the source exists. + return False + return True + + @classmethod + def get_output_dir(cls, session): + return None + + def needs_raw_data(self): + return False + + class NiiExporter(SeriesExporter): """Export a series to nifti format with datman-style names. """ From 7f8f8a7da36cd7eaf8f7392addde3858d44caa9f Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 11 Oct 2023 19:28:23 -0400 Subject: [PATCH 03/10] [ENH] Update DBExporter to handle xnat shared sessions --- datman/exporters.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/datman/exporters.py b/datman/exporters.py index 8eed495e..4de9daa9 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -857,11 +857,36 @@ def make_scan(self, file_stem): logger.error(f"Failed adding scan {file_stem} to dashboard " f"with error: {exc}") return - + if self.experiment.is_shared(): + self._make_linked(scan) self._add_bids_scan_name(scan, file_stem) self._add_side_car(scan, file_stem) self._update_conversion_errors(scan, file_stem) + def _make_linked(self, scan): + try: + source_session = datman.dashboard.get_session(self.experiment.name) + except datman.dashboard.DashboardException as exc: + logger.error( + f"Failed to link shared scan {scan} to source " + f"{self.experiment.name}. Reason - {exc}" + ) + return + matches = [ + source_scan for source_scan in source_session.scans + if (source_scan.series == scan.series and + source_scan.tag == source_scan.tag) + ] + if not matches or len(matches) > 1: + logger.error( + f"Failed to link shared scan {scan} to {self.experiment.name}." + " Reason - Unable to find source scan database record." + ) + return + + scan.source_id = matches[0].id + scan.save() + def _add_bids_scan_name(self, scan, dm_stem): """Add a bids format file name to a series in the QC database. From 9584af84addc5e2d50ed5c1ee1f30417a82e4469 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 11 Oct 2023 19:30:24 -0400 Subject: [PATCH 04/10] [ENH] Allow dm_xnat_extract.py to use SharedExporter --- bin/dm_xnat_extract.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/bin/dm_xnat_extract.py b/bin/dm_xnat_extract.py index 635debd8..1c1bb885 100755 --- a/bin/dm_xnat_extract.py +++ b/bin/dm_xnat_extract.py @@ -153,7 +153,7 @@ def main(): session = datman.scan.Scan(ident, config, bids_root=args.bids_out) - if xnat_experiment.resource_files: + if xnat_experiment.resource_files and not xnat_experiment.is_shared(): export_resources(session.resource_path, xnat, xnat_experiment, dry_run=args.dry_run) @@ -611,7 +611,11 @@ def make_session_exporters(config, session, experiment, bids_opts=None, list: Returns a list of :obj:`datman.exporters.Exporter` for the desired session export formats. """ - formats = get_session_formats(bids_opts=bids_opts, ignore_db=ignore_db) + formats = get_session_formats( + bids_opts=bids_opts, + shared=experiment.is_shared(), + ignore_db=ignore_db + ) exporters = [] for exp_format in formats: @@ -623,12 +627,14 @@ def make_session_exporters(config, session, experiment, bids_opts=None, return exporters -def get_session_formats(bids_opts=None, ignore_db=False): +def get_session_formats(bids_opts=None, shared=False, ignore_db=False): """Get the string identifiers for all session exporters that are needed. Args: bids_opts (:obj:`BidsOptions`, optional): dcm2bids settings to be used if exporting to BIDS format. Defaults to None. + shared (bool, optional): Whether to treat the session as a + shared XNAT experiment. Defaults to False. ignore_db (bool, optional): If True, datman's QC dashboard will not be updated. Defaults to False. @@ -636,8 +642,13 @@ def get_session_formats(bids_opts=None, ignore_db=False): list: a list of string keys that should be used to make exporters. """ formats = [] - if bids_opts: + if shared: + formats.append("shared") + elif bids_opts: + # Only do 'bids' format if not a shared session. formats.append("bids") + + if bids_opts: formats.append("nii_link") if not ignore_db: formats.append("db") From 85b3d015d881e493b5b5d73a60b228877a96895e Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 12 Oct 2023 14:53:55 -0400 Subject: [PATCH 05/10] [FIX] Source scan was being compared to itself (oops!) --- datman/exporters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datman/exporters.py b/datman/exporters.py index 4de9daa9..74093a6e 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -875,7 +875,7 @@ def _make_linked(self, scan): matches = [ source_scan for source_scan in source_session.scans if (source_scan.series == scan.series and - source_scan.tag == source_scan.tag) + source_scan.tag == scan.tag) ] if not matches or len(matches) > 1: logger.error( From b4620a118683bbe3ad2485cd9616ef48c8851001 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Mon, 16 Oct 2023 13:48:57 -0400 Subject: [PATCH 06/10] [FIX] Allow nifti list to update after scan object is created --- datman/scan.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/datman/scan.py b/datman/scan.py index 8030a131..a991e6b4 100644 --- a/datman/scan.py +++ b/datman/scan.py @@ -131,12 +131,14 @@ def __init__(self, subject_id, config, bids_root=None): self.resource_path = self.__get_path( "resources", config, session=True) - self.niftis = self.__get_series(self.nii_path, [".nii", ".nii.gz"]) - self.__nii_dict = self.__make_dict(self.niftis) self.nii_tags = list(self.__nii_dict.keys()) + @property + def niftis(self): + return self.__get_series(self.nii_path, ['nii', '.nii.gz']) + def _get_ident(self, subid): subject_id = self.__check_session(subid) try: From b61671fb3e61ee27c47de118da455f3f8e446bf6 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Mon, 16 Oct 2023 21:01:05 -0400 Subject: [PATCH 07/10] [FIX] Create proper QC-file output names --- datman/exporters.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index 74093a6e..2805b739 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -1136,8 +1136,11 @@ def find_dm_files(self, dir_type, name_map=None): self.source_session.id_plus_session ) for match in glob(os.path.join(source_dir, f"{source_name}*")): - ext = get_extension(match) - name_map[match] = os.path.join(dest_dir, name + ext) + fname = os.path.basename(match).replace( + self.source_session.id_plus_session, + self.session.id_plus_session + ) + name_map[match] = os.path.join(dest_dir, fname) return name_map From d45703cc6e38b048bb031320494758d447cfead0 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Tue, 17 Oct 2023 16:26:04 -0400 Subject: [PATCH 08/10] [FIX] Allow qc images to display correctly, find bids-only files - The manifest files linked from the source session were causing incorrect paths to be generated in the dashboard, causing metrics to appear as broken images. This is resolved now, by omitting them from the symlink name_map - Files that could not be traditionally exported directly from xnat (i.e. files that could only be exported with dcm2niix) were being missed when searching datman-style paths. This is fixed now, and all valid datman-style names with matching tags will be detected. --- datman/exporters.py | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index 2805b739..ff57372a 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -25,7 +25,7 @@ from datman.exceptions import (UndefinedSetting, DashboardException, ExportException) from datman.scanid import (parse, parse_bids_filename, ParseException, - make_filename, KCNIIdentifier) + parse_filename, make_filename, KCNIIdentifier) from datman.utils import (run, make_temp_directory, get_extension, filter_niftis, find_tech_notes, read_blacklist, get_relative_source, read_json, write_json) @@ -1026,6 +1026,8 @@ def __init__(self, config, session, experiment, bids_opts=None, **kwargs): if not bids_opts: dm_dirs.append('nii_path') + self.tags = config.get_tags(site=session.site) + super().__init__(config, session, experiment, **kwargs) self.name_map = self.make_name_map(dm_dirs, use_bids=bids_opts) @@ -1126,21 +1128,29 @@ def find_dm_files(self, dir_type, name_map=None): source_dir = getattr(self.source_session, dir_type) dest_dir = getattr(self.session, dir_type) - for scan in self.experiment.scans: - for name in scan.names: - if read_blacklist(scan=name, config=self.config): - continue - - source_name = name.replace( - self.session.id_plus_session, - self.source_session.id_plus_session + for item in glob(os.path.join(source_dir, "*")): + try: + _, tag, _, _ = parse_filename(item) + except ParseException: + logger.debug( + f"Ignoring invalid file name {item} in {source_dir}" ) - for match in glob(os.path.join(source_dir, f"{source_name}*")): - fname = os.path.basename(match).replace( - self.source_session.id_plus_session, - self.session.id_plus_session - ) - name_map[match] = os.path.join(dest_dir, fname) + continue + + if tag not in self.tags: + # Found a valid scan name but with a tag not used by dest study + continue + + if dir_type == 'qc_path' and item.endswith('_manifest.json'): + # Filter out manifest files. These should be regenerated + # by dm_qc_report for the dest session. + continue + + fname = os.path.basename(item).replace( + self.source_session.id_plus_session, + self.session.id_plus_session + ) + name_map[item] = os.path.join(dest_dir, fname) return name_map From 09bb73f625be800d11b902e3468b7684784420fd Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Tue, 17 Oct 2023 17:19:32 -0400 Subject: [PATCH 09/10] [FIX] Stop DBExporter from missing fmaps and other bids-only files --- datman/exporters.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index ff57372a..05f62c1b 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -662,11 +662,11 @@ def __init__(self, config, session, experiment, **kwargs): self.study_resource_path = study_resource_dir self.resources_path = resources_dir self.date = experiment.date - self.names = self.get_scan_names(session, experiment) super().__init__(config, session, experiment, **kwargs) - def get_scan_names(self, session, experiment): - """Gets list of datman-style scan names for a session. + @property + def names(self): + """Gets list of valid datman-style scan names for a session. Returns: :obj:`dict`: A dictionary of datman style scan names mapped to @@ -675,17 +675,17 @@ def get_scan_names(self, session, experiment): """ names = {} # use experiment.scans, so dashboard can report scans that didnt export - for scan in experiment.scans: + for scan in self.experiment.scans: for name in scan.names: - names[name] = self.get_bids_name(name, session) + names[name] = self.get_bids_name(name, self.session) # Check the actual folder contents as well, in case symlinked scans # exist that werent named on XNAT - for nii in session.niftis: + for nii in self.session.niftis: fname = nii.file_name.replace(nii.ext, "") if fname in names: continue - names[fname] = self.get_bids_name(fname, session) + names[fname] = self.get_bids_name(fname, self.session) return names From 525dfdbbc0992331559e3668d08eba3a40aee249 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Tue, 17 Oct 2023 18:37:48 -0400 Subject: [PATCH 10/10] [FIX] Strip off optional 'hours' from redcap date for DEEPPI --- bin/dm_redcap_scan_completed.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/dm_redcap_scan_completed.py b/bin/dm_redcap_scan_completed.py index d54528ac..796845b3 100755 --- a/bin/dm_redcap_scan_completed.py +++ b/bin/dm_redcap_scan_completed.py @@ -94,6 +94,8 @@ def add_session_redcap(record, record_key): return session_date = record[get_setting('RedcapDate', default='date')] + # Strip off optional 'hours' field' + session_date = session_date.split(" ")[0] try: session = dashboard.get_session(ident, date=session_date, create=True)