Skip to content

Commit

Permalink
FIX: Make lists and dicts hashable
Browse files Browse the repository at this point in the history
Resolves: #683.
  • Loading branch information
oesteban committed Apr 14, 2022
1 parent 5ea3103 commit 80e9d2e
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 43 deletions.
86 changes: 43 additions & 43 deletions bids/layout/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -591,6 +591,10 @@ def get(self, return_type='object', target=None, scope='all',
A list of BIDSFiles (default) or strings (see return_type).
"""

if return_type not in ("object", "file", "filename", "id", "dir"):
raise ValueError("Invalid return_type specified (must be one "
"of 'object', 'file', 'filename', 'id', or 'dir'.")

if absolute_paths is False:
absolute_path_deprecation_warning()

Expand Down Expand Up @@ -637,6 +641,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:
Expand All @@ -663,49 +670,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')

Expand Down
21 changes: 21 additions & 0 deletions bids/layout/tests/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
20 changes: 20 additions & 0 deletions bids/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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()):
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ install_requires =
bids-validator
num2words
click
frozendict
tests_require =
pytest >=3.3
mock
Expand Down

0 comments on commit 80e9d2e

Please sign in to comment.