diff --git a/bids/layout/layout.py b/bids/layout/layout.py index cfd88179a..4c5e51502 100644 --- a/bids/layout/layout.py +++ b/bids/layout/layout.py @@ -15,7 +15,7 @@ from sqlalchemy.sql.expression import cast from bids_validator import BIDSValidator -from ..utils import listify, natural_sort +from ..utils import listify, natural_sort, hashablefy from ..external import inflect from ..exceptions import ( BIDSEntityError, @@ -591,6 +591,13 @@ def get(self, return_type='object', target=None, scope='all', A list of BIDSFiles (default) or strings (see return_type). """ + if ( + not return_type.startswith(("obj", "file")) + and return_type not in ("id", "dir") + ): + raise ValueError(f"Invalid return_type <{return_type}> specified (must be one " + "of 'object', 'file', 'filename', 'id', or 'dir').") + if absolute_paths is False: absolute_path_deprecation_warning() @@ -637,6 +644,9 @@ def get(self, return_type='object', target=None, scope='all', message = "Valid targets are: {}".format(potential) raise TargetError(("Unknown target '{}'. " + message) .format(target)) + elif target is None and return_type in ['id', 'dir']: + raise TargetError('If return_type is "id" or "dir", a valid ' + 'target entity must also be specified.') results = [] for l in layouts: @@ -663,49 +673,42 @@ def get(self, return_type='object', target=None, scope='all', results[i] = fi if return_type.startswith('file'): - results = natural_sort([f.path for f in results]) - - elif return_type in ['id', 'dir']: - if target is None: - raise TargetError('If return_type is "id" or "dir", a valid ' - 'target entity must also be specified.') - - metadata = target not in self.get_entities(metadata=False) - - if return_type == 'id': - ent_iter = (x.get_entities(metadata=metadata) for x in results) - results = list({ - ents[target] for ents in ent_iter if target in ents - }) - - elif return_type == 'dir': - template = entities[target].directory - if template is None: - raise ValueError('Return type set to directory, but no ' - 'directory template is defined for the ' - 'target entity (\"%s\").' % target) - # Construct regex search pattern from target directory template - # On Windows, the regex won't compile if, e.g., there is a folder starting with "U" on the path. - # Converting to a POSIX path with forward slashes solves this. - template = self._root.as_posix() + template - to_rep = re.findall(r'{(.*?)\}', template) - for ent in to_rep: - patt = entities[ent].pattern - template = template.replace('{%s}' % ent, patt) - # Avoid matching subfolders. We are working with POSIX paths here, so we explicitly use "/" - # as path separator. - template += r'[^/]*$' - matches = [ - f.dirname if absolute_paths else str(f._dirname.relative_to(self._root)) # noqa: E501 - for f in results - if re.search(template, f._dirname.as_posix()) - ] - - results = natural_sort(list(set(matches))) + return natural_sort([f.path for f in results]) - else: - raise ValueError("Invalid return_type specified (must be one " - "of 'tuple', 'filename', 'id', or 'dir'.") + metadata = target not in self.get_entities(metadata=False) + + if return_type == 'id': + ent_iter = ( + hashablefy(x.get_entities(metadata=metadata)) + for x in results + ) + results = list({ + ents[target] for ents in ent_iter if target in ents + }) + elif return_type == 'dir': + template = entities[target].directory + if template is None: + raise ValueError('Return type set to directory, but no ' + 'directory template is defined for the ' + 'target entity (\"%s\").' % target) + # Construct regex search pattern from target directory template + # On Windows, the regex won't compile if, e.g., there is a folder starting with "U" on the path. + # Converting to a POSIX path with forward slashes solves this. + template = self._root.as_posix() + template + to_rep = re.findall(r'{(.*?)\}', template) + for ent in to_rep: + patt = entities[ent].pattern + template = template.replace('{%s}' % ent, patt) + # Avoid matching subfolders. We are working with POSIX paths here, so we explicitly use "/" + # as path separator. + template += r'[^/]*$' + matches = [ + f.dirname if absolute_paths else str(f._dirname.relative_to(self._root)) # noqa: E501 + for f in results + if re.search(template, f._dirname.as_posix()) + ] + + results = natural_sort(list(set(matches))) else: results = natural_sort(results, 'path') diff --git a/bids/layout/tests/test_layout.py b/bids/layout/tests/test_layout.py index 7d1dd526b..17495602f 100644 --- a/bids/layout/tests/test_layout.py +++ b/bids/layout/tests/test_layout.py @@ -548,6 +548,27 @@ def test_get_tr(layout_7t_trt): assert tr == 4.0 +def test_get_nonhashable_metadata(layout_ds117): + """Test nonhashable metadata values (#683).""" + assert layout_ds117.get_IntendedFor(subject=['01'])[0] == ( + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-01_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-02_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-03_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-04_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-05_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-06_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-07_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-08_bold.nii.gz", + "ses-mri/func/sub-01_ses-mri_task-facerecognition_run-09_bold.nii.gz", + ) + + landmarks = layout_ds117.get_AnatomicalLandmarkCoordinates(subject=['01'])[0] + assert landmarks["Nasion"] == (43, 111, 95) + assert landmarks["LPA"] == (140, 74, 16) + assert landmarks["RPA"] == (143, 74, 173) + + + def test_to_df(layout_ds117): # Only filename entities df = layout_ds117.to_df() diff --git a/bids/utils.py b/bids/utils.py index c8db9515e..3c7429534 100644 --- a/bids/utils.py +++ b/bids/utils.py @@ -3,6 +3,16 @@ import re import os from pathlib import Path +from frozendict import frozendict as _frozendict + + +# Monkeypatch to print out frozendicts *as if* they were dictionaries. +class frozendict(_frozendict): + """A hashable dictionary type.""" + + def __repr__(self): + """Override frozendict representation.""" + return repr({k: v for k, v in self.items()}) def listify(obj): @@ -11,6 +21,16 @@ def listify(obj): return obj if isinstance(obj, (list, tuple, type(None))) else [obj] +def hashablefy(obj): + ''' Make dictionaries and lists hashable or raise. ''' + if isinstance(obj, list): + return tuple([hashablefy(o) for o in obj]) + + if isinstance(obj, dict): + return frozendict({k: hashablefy(v) for k, v in obj.items()}) + return obj + + def matches_entities(obj, entities, strict=False): ''' Checks whether an object's entities match the input. ''' if strict and set(obj.entities.keys()) != set(entities.keys()): diff --git a/setup.cfg b/setup.cfg index bdf8953b1..1628d0fcc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,6 +31,7 @@ install_requires = bids-validator num2words click + frozendict tests_require = pytest >=3.3 mock