From dc4b56bce93b8472909bc61f39f18fb041743bde Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Tue, 14 Jan 2025 18:29:14 -0500 Subject: [PATCH 1/9] [FIX] Ensure shared experiments use name associated with current project --- datman/xnat.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/datman/xnat.py b/datman/xnat.py index e28dbbef..38288717 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -1319,9 +1319,14 @@ def __init__(self, project, subject_name, experiment_json): self.subject = subject_name self.uid = self._get_field("UID") self.id = self._get_field("ID") - self.name = self._get_field("label") self.date = self._get_field("date") + if self.is_shared(): + self.name = [label for label in self.get_alt_labels() + if self.subject in label][0] + else: + self.name = self._get_field("label") + # Scan attributes self.scans = self._get_scans() self.scan_UIDs = self._get_scan_UIDs() @@ -1594,20 +1599,21 @@ def assign_scan_names(self, config, ident): 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. + """Check if the experiment is shared from another project. """ - if self.subject in self.name: + alt_names = self.get_alt_labels() + if not alt_names: return False - share_entry = self._get_contents('sharing/share') - if not share_entry: - return False + return any([self.subject in label for label in alt_names]) - return self.subject in share_entry[0][0]['data_fields']['label'] + def get_alt_labels(self): + """Find the names for all shared copies of the XNAT experiment. + """ + shared = self._get_contents("sharing/share") + if not shared: + return [] + return [item['data_fields']['label'] for item in shared[0]] def __str__(self): return f"" From 30d66ed3871288b81c1a1464985e6cede27e370f Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 15 Jan 2025 13:14:10 -0500 Subject: [PATCH 2/9] [REF] Delete SharedExporter (sigh) --- datman/exporters.py | 227 -------------------------------------------- 1 file changed, 227 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index 05f62c1b..38517168 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -999,233 +999,6 @@ 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') - - 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) - - 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 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}" - ) - 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 - - 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 0288fd0038ec074365d280a9c53d8364f4ce367c Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Wed, 15 Jan 2025 13:23:43 -0500 Subject: [PATCH 3/9] [REF] Remove references to shared sessions --- bin/dm_xnat_extract.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/bin/dm_xnat_extract.py b/bin/dm_xnat_extract.py index 021e6b6e..75e3ace4 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 and not xnat_experiment.is_shared(): + if xnat_experiment.resource_files: export_resources(session.resource_path, xnat, xnat_experiment, dry_run=args.dry_run) @@ -380,8 +380,8 @@ def get_experiment_identifier(config, project, experiment_id): try: ident = validate_subject_id(experiment_id, config) except datman.scanid.ParseException: - logger.error(f"Invalid XNAT experiment ID {experiment_id} in project " - f"{project}. Please update XNAT with correct ID.") + logger.error(f"Invalid experiment ID {experiment_id} in project " + f"{project}.") return if ident.session is None and not datman.scanid.is_phantom(ident): @@ -613,7 +613,6 @@ def make_session_exporters(config, session, experiment, bids_opts=None, """ formats = get_session_formats( bids_opts=bids_opts, - shared=experiment.is_shared(), ignore_db=ignore_db ) @@ -627,14 +626,12 @@ def make_session_exporters(config, session, experiment, bids_opts=None, return exporters -def get_session_formats(bids_opts=None, shared=False, ignore_db=False): +def get_session_formats(bids_opts=None, 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. @@ -642,13 +639,8 @@ def get_session_formats(bids_opts=None, shared=False, ignore_db=False): list: a list of string keys that should be used to make exporters. """ formats = [] - 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("bids") formats.append("nii_link") if not ignore_db: formats.append("db") From 807bbb3e4a2de668b9de287955833b68704cf258 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 16:18:33 -0500 Subject: [PATCH 4/9] [FIX] Add code to rename downloaded scans for XNAT-shared sessions --- datman/xnat.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/datman/xnat.py b/datman/xnat.py index 38288717..197c4781 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -8,6 +8,7 @@ import re import tempfile import time +import shutil import urllib.parse from abc import ABC from xml.etree import ElementTree @@ -1324,8 +1325,10 @@ def __init__(self, project, subject_name, experiment_json): if self.is_shared(): self.name = [label for label in self.get_alt_labels() if self.subject in label][0] + self.source_name = self._get_field("label") else: self.name = self._get_field("label") + self.source_name = self.name # Scan attributes self.scans = self._get_scans() @@ -1627,6 +1630,8 @@ def __init__(self, experiment, scan_json): self.project = experiment.project self.subject = experiment.subject self.experiment = experiment.name + self.shared = experiment.is_shared() + self.source_name = experiment.source_name self.raw_json = scan_json self.uid = self._get_field("UID") self.series = self._get_field("ID") @@ -1821,6 +1826,9 @@ def download(self, xnat_conn, output_dir): f"{dicom_zip}") os.remove(dicom_zip) + if self.shared: + self._fix_download_name(output_dir) + dicom_file = self._find_first_dicom(output_dir) try: @@ -1868,6 +1876,27 @@ def _find_series_dir(self, search_dir): return search_dir return found[0] + def _fix_download_name(self, output_dir): + """Rename a downloaded XNAT-shared scan to match the expected label. + """ + orig_dir = os.path.join(output_dir, self.source_name) + try: + os.rename(orig_dir, orig_dir.replace(self.source_name, self.name)) + except OSError: + for root, dirs, _ in os.walk(orig_dir): + for item in dirs: + try: + os.rename(os.path.join(root, item), + os.path.join( + root.replace(self.source_name, self.name), + item) + ) + except OSError: + pass + else: + shutil.rmtree(orig_dir) + return + def __str__(self): return f"" From a701b478ac28c6df3183ef8e5613697e84512c24 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 16:27:22 -0500 Subject: [PATCH 5/9] [FIX] Fix var names --- datman/xnat.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datman/xnat.py b/datman/xnat.py index 197c4781..fbc8af92 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -1881,14 +1881,14 @@ def _fix_download_name(self, output_dir): """ orig_dir = os.path.join(output_dir, self.source_name) try: - os.rename(orig_dir, orig_dir.replace(self.source_name, self.name)) + os.rename(orig_dir, orig_dir.replace(self.source_name, self.experiment)) except OSError: for root, dirs, _ in os.walk(orig_dir): for item in dirs: try: os.rename(os.path.join(root, item), os.path.join( - root.replace(self.source_name, self.name), + root.replace(self.source_name, self.experiment), item) ) except OSError: From bd13460b6bf904717129308baae6ce4c912b3af3 Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 16:29:38 -0500 Subject: [PATCH 6/9] [FIX] Fix formatting issues --- datman/xnat.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/datman/xnat.py b/datman/xnat.py index fbc8af92..fabd3d4b 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -1631,7 +1631,7 @@ def __init__(self, experiment, scan_json): self.subject = experiment.subject self.experiment = experiment.name self.shared = experiment.is_shared() - self.source_name = experiment.source_name + self.source_experiment = experiment.source_name self.raw_json = scan_json self.uid = self._get_field("UID") self.series = self._get_field("ID") @@ -1879,16 +1879,21 @@ def _find_series_dir(self, search_dir): def _fix_download_name(self, output_dir): """Rename a downloaded XNAT-shared scan to match the expected label. """ - orig_dir = os.path.join(output_dir, self.source_name) + orig_dir = os.path.join(output_dir, self.source_experiment) try: - os.rename(orig_dir, orig_dir.replace(self.source_name, self.experiment)) + os.rename(orig_dir, + orig_dir.replace( + self.source_experiment, + self.experiment)) except OSError: for root, dirs, _ in os.walk(orig_dir): for item in dirs: try: os.rename(os.path.join(root, item), os.path.join( - root.replace(self.source_name, self.experiment), + root.replace( + self.source_experiment, + self.experiment), item) ) except OSError: From b8d3bb732585fd7a29e028398acceca37ce8040e Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 19:39:21 -0500 Subject: [PATCH 7/9] [REF] Re-add more informative log message --- bin/dm_xnat_extract.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/dm_xnat_extract.py b/bin/dm_xnat_extract.py index 75e3ace4..b20f07c9 100755 --- a/bin/dm_xnat_extract.py +++ b/bin/dm_xnat_extract.py @@ -380,8 +380,8 @@ def get_experiment_identifier(config, project, experiment_id): try: ident = validate_subject_id(experiment_id, config) except datman.scanid.ParseException: - logger.error(f"Invalid experiment ID {experiment_id} in project " - f"{project}.") + logger.error(f"Invalid XNAT experiment ID {experiment_id} in project " + f"{project}. Please update XNAT with correct ID.") return if ident.session is None and not datman.scanid.is_phantom(ident): From a49bc1bf8c43452a5ab3d7817c939f6fea63ca0f Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 20:08:06 -0500 Subject: [PATCH 8/9] [PEP8] Fix formatting issues --- datman/exporters.py | 7 +++---- datman/xnat.py | 15 ++++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index 38517168..69c7d37e 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -22,10 +22,9 @@ import datman.config import datman.dashboard import datman.scan -from datman.exceptions import (UndefinedSetting, DashboardException, - ExportException) -from datman.scanid import (parse, parse_bids_filename, ParseException, - parse_filename, make_filename, KCNIIdentifier) +from datman.exceptions import UndefinedSetting, DashboardException +from datman.scanid import (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) diff --git a/datman/xnat.py b/datman/xnat.py index fabd3d4b..0d00184f 100644 --- a/datman/xnat.py +++ b/datman/xnat.py @@ -506,8 +506,8 @@ def make_experiment(self, project, subject, experiment): """ url = ( - f"{self.server}/data/archive/projects/{project}/subjects/{subject}/" - f"experiments/{experiment}?xsiType=xnat:mrSessionData") + f"{self.server}/data/archive/projects/{project}/subjects/" + f"{subject}/experiments/{experiment}?xsiType=xnat:mrSessionData") try: self._make_xnat_put(url) except requests.exceptions.RequestException as e: @@ -531,12 +531,13 @@ def get_scan_ids(self, project, subject, experiment): """ logger.debug( f"Querying XNAT server {self.server} for scan IDs belonging to " - f"experiment {experiment} of subject {subject} in project {project}" + f"experiment {experiment} of subject {subject} in project " + f"{project}" ) url = ( - f"{self.server}/data/archive/projects/{project}/subjects/{subject}/" - f"experiments/{experiment}/scans/?format=json") + f"{self.server}/data/archive/projects/{project}/subjects/" + f"{subject}/experiments/{experiment}/scans/?format=json") try: result = self._make_xnat_query(url) @@ -1689,7 +1690,7 @@ def set_tag(self, tag_map): search_target = self.type else: raise KeyError( - f"Missing keys 'SeriesDescription' or 'XnatType'" + "Missing keys 'SeriesDescription' or 'XnatType'" " for Pattern!") if isinstance(regex, list): @@ -1895,7 +1896,7 @@ def _fix_download_name(self, output_dir): self.source_experiment, self.experiment), item) - ) + ) except OSError: pass else: From 722312421d565cb3d2286bfcaee6e82687cddf2e Mon Sep 17 00:00:00 2001 From: Dawn Smith Date: Thu, 16 Jan 2025 20:54:32 -0500 Subject: [PATCH 9/9] [FIX] Ensure ID modifications are applied to shared sessions --- datman/exporters.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/datman/exporters.py b/datman/exporters.py index 69c7d37e..db326d42 100644 --- a/datman/exporters.py +++ b/datman/exporters.py @@ -22,7 +22,8 @@ import datman.config import datman.dashboard import datman.scan -from datman.exceptions import UndefinedSetting, DashboardException +from datman.exceptions import (UndefinedSetting, DashboardException, + ConfigException) from datman.scanid import (parse_bids_filename, ParseException, make_filename, KCNIIdentifier) from datman.utils import (run, make_temp_directory, get_extension, @@ -857,18 +858,19 @@ def make_scan(self, file_stem): f"with error: {exc}") return if self.experiment.is_shared(): - self._make_linked(scan) + source_session = self._get_source_session() + self._make_linked(scan, source_session) 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): + def _make_linked(self, scan, source_session): try: - source_session = datman.dashboard.get_session(self.experiment.name) + source_session = datman.dashboard.get_session(source_session) except datman.dashboard.DashboardException as exc: logger.error( f"Failed to link shared scan {scan} to source " - f"{self.experiment.name}. Reason - {exc}" + f"{source_session}. Reason - {exc}" ) return matches = [ @@ -878,7 +880,7 @@ def _make_linked(self, scan): ] if not matches or len(matches) > 1: logger.error( - f"Failed to link shared scan {scan} to {self.experiment.name}." + f"Failed to link shared scan {scan} to {source_session}." " Reason - Unable to find source scan database record." ) return @@ -886,6 +888,20 @@ def _make_linked(self, scan): scan.source_id = matches[0].id scan.save() + def _get_source_session(self): + """Get the ID of the source experiment for a shared XNATExperiment.""" + try: + config = datman.config.config(study=self.experiment.source_name) + except ConfigException: + return self.experiment.source_name + + try: + id_map = config.get_key('IdMap') + except UndefinedSetting: + return self.experiment.source_name + + return str(datman.scanid.parse(self.experiment.source_name, id_map)) + def _add_bids_scan_name(self, scan, dm_stem): """Add a bids format file name to a series in the QC database.