Skip to content

Commit

Permalink
Fix lock state on query and __repr__ of resources
Browse files Browse the repository at this point in the history
  • Loading branch information
gacou54 committed Aug 30, 2023
1 parent beaba0e commit 4f118f9
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 45 deletions.
1 change: 0 additions & 1 deletion pyorthanc/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from pyorthanc.resources.study import Study
from .async_client import AsyncOrthanc
from .client import Orthanc
from .filtering import build_patient_forest, find, trim_patients
Expand Down
28 changes: 19 additions & 9 deletions pyorthanc/find.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ def query_orthanc(client: Orthanc,
labels_constraint: str = 'All',
limit: int = DEFAULT_RESOURCES_LIMIT,
since: int = 0,
retrieve_all_resources: bool = True) -> List[Resource]:
retrieve_all_resources: bool = True,
lock: bool = False) -> List[Resource]:
"""
Parameters
Expand All @@ -215,6 +216,8 @@ def query_orthanc(client: Orthanc,
Show only the resources since the provided index (in conjunction with "limit").
retrieve_all_resources
Retrieve all resources since the index specified in the "since" parameter.
lock
if `True`, lock the resource state at lookup (useful for minimising the number of HTTP calls).
Returns
-------
Expand Down Expand Up @@ -267,16 +270,23 @@ def query_orthanc(client: Orthanc,
results = client.post_tools_find(data)

if level == 'Patient':
return [Patient(i['ID'], client, i) for i in results]

if level == 'Study':
return [Study(i['ID'], client, i) for i in results]
resources = [Patient(i['ID'], client, lock=lock) for i in results]
elif level == 'Study':
resources = [Study(i['ID'], client, lock=lock) for i in results]
elif level == 'Series':
resources = [Series(i['ID'], client, lock=lock) for i in results]
elif level == 'Instance':
resources = [Instance(i['ID'], client, lock=lock) for i in results]
else:
raise ValueError(f"Unknown level ['Patient', 'Study', 'Series', 'Instance'], got {level}")

if level == 'Series':
return [Series(i['ID'], client, i) for i in results]
if lock:
for resource in resources:
# This loads the state in memory. Since lock=True,
# subsequent queries on resource will use the local state
resource.get_main_information()

if level == 'Instance':
return [Instance(i['ID'], client, i) for i in results]
return resources


def _validate_labels_constraint(labels_constraint: str) -> None:
Expand Down
3 changes: 0 additions & 3 deletions pyorthanc/resources/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,3 @@ def anonymize(self, remove: List = None, replace: Dict = None, keep: List = None
def get_pydicom(self) -> pydicom.FileDataset:
"""Retrieve a pydicom.FileDataset object corresponding to the instance."""
return util.get_pydicom(self.client, self.id_)

def __repr__(self):
return f'Instance(identifier={self.id_})'
3 changes: 0 additions & 3 deletions pyorthanc/resources/patient.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,3 @@ def remove_empty_studies(self) -> None:
study.remove_empty_series()

self._child_resources = [study for study in self._child_resources if study._child_resources != []]

def __repr__(self):
return f'Patient(PatientID={self.patient_id}, identifier={self.id_})'
6 changes: 6 additions & 0 deletions pyorthanc/resources/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,11 @@ def identifier(self) -> str:
"""
return self.id_

def get_main_information(self):
raise NotImplementedError

def __eq__(self, other: 'Resource') -> bool:
return self.id_ == other.id_

def __repr__(self):
return f'{self.__class__.__name__}({self.id_})'
3 changes: 0 additions & 3 deletions pyorthanc/resources/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ def get_zip(self) -> bytes:
"""
return self.client.get_series_id_archive(self.id_)

def __repr__(self):
return f'Series(identifier={self.id_})'

def remove_empty_instances(self) -> None:
if self._child_resources is not None:
self._child_resources = [i for i in self._child_resources if i is not None]
3 changes: 0 additions & 3 deletions pyorthanc/resources/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,3 @@ def remove_empty_series(self) -> None:
series.remove_empty_instances()

self._child_resources = [series for series in self._child_resources if series._child_resources != []]

def __repr__(self):
return f'Study(StudyId={self.study_id}, identifier={self.id_})'
54 changes: 31 additions & 23 deletions tests/test_find.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,35 +84,37 @@ def test_find_instance(client_with_data_and_labels, query, labels, expected):
assert [instance.id_ for instance in result] == expected


@pytest.mark.parametrize('level, query, labels, labels_constraint, limit, since, retrieve_all_resources, expected', [
@pytest.mark.parametrize('level, query, labels, labels_constraint, limit, since, retrieve_all_resources, lock, expected', [
# On level
('Patient', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Study', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_study.IDENTIFIER]),
('Series', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b', 'c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
('Instance', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, ['22dcf059-8fd3ade7-efb39ca3-7f46b248-0200abc9', 'da2024f5-606f9e83-41b012bb-9dced1ea-77bcd599', '348befe7-5be5ff53-70120381-3baa0cc2-e4e04220']),
('Patient', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Study', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_study.IDENTIFIER]),
('Series', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b', 'c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
('Instance', None, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, ['22dcf059-8fd3ade7-efb39ca3-7f46b248-0200abc9', 'da2024f5-606f9e83-41b012bb-9dced1ea-77bcd599', '348befe7-5be5ff53-70120381-3baa0cc2-e4e04220']),
# On query
('Patient', {}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', {'PatientID': '*'}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', {'PatientID': 'NOT_EXISTING_PATIENT'}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, []),
('Patient', {}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', {'PatientID': '*'}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', {'PatientID': 'NOT_EXISTING_PATIENT'}, None, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, []),
# On labels
('Patient', None, LABEL_PATIENT, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, '', 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, [], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, ['NOT_EXISTING_LABEL'], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, []),
('Patient', None, LABEL_PATIENT, 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, '', 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, [], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, ['NOT_EXISTING_LABEL'], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, []),
# On labels_constraint
('Patient', None, [LABEL_PATIENT], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT, 'BAD_LABEL'], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, []),
('Patient', None, [LABEL_PATIENT, 'BAD_LABEL'], 'Any', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT], 'None', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, []),
('Patient', None, None, 'None', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT, 'BAD_LABEL'], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, []),
('Patient', None, [LABEL_PATIENT, 'BAD_LABEL'], 'Any', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
('Patient', None, [LABEL_PATIENT], 'None', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, []),
('Patient', None, None, 'None', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, False, [a_patient.IDENTIFIER]),
# On limit and since
('Series', None, None, 'All', DEFAULT_RESOURCES_LIMIT, 1, False, ['c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
('Series', None, None, 'All', 1, DEFAULT_SINCE, False, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b']),
('Series', None, None, 'All', 1, DEFAULT_SINCE, True, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b', 'c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
('Series', None, None, 'All', DEFAULT_RESOURCES_LIMIT, 1, False, False, ['c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
('Series', None, None, 'All', 1, DEFAULT_SINCE, False, False, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b']),
('Series', None, None, 'All', 1, DEFAULT_SINCE, True, False, ['60108266-ece4d8f7-7b028286-a7b61f25-c6d33f0b', 'c4c1fcc9-ae63f793-40cbcf25-fbd3efe5-ad72ff06', 'e2a7df26-99673e0f-05aa84cd-c89677c0-634a2a96']),
# On lock
('Patient', None, [LABEL_PATIENT], 'All', DEFAULT_RESOURCES_LIMIT, DEFAULT_SINCE, False, True, [a_patient.IDENTIFIER]),
])
def test_query_orthanc(client_with_data_and_labels, level, query, labels, labels_constraint, limit, since, retrieve_all_resources, expected):
def test_query_orthanc(client_with_data_and_labels, level, query, labels, labels_constraint, limit, since, retrieve_all_resources, lock, expected):
result = query_orthanc(
client=client_with_data_and_labels,
level=level,
Expand All @@ -121,10 +123,16 @@ def test_query_orthanc(client_with_data_and_labels, level, query, labels, labels
labels_constraint=labels_constraint,
limit=limit,
since=since,
retrieve_all_resources=retrieve_all_resources
retrieve_all_resources=retrieve_all_resources,
lock=lock
)

assert [resource.id_ for resource in result] == expected
for resource in result:
if lock:
assert isinstance(resource._information, dict)
else:
assert resource._information is None


@pytest.mark.parametrize('level, labels_constraint', [
Expand Down
1 change: 1 addition & 0 deletions tests/test_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def test_attributes(instance):

assert '0008,0012' in instance.tags.keys()
assert 'Value' in instance.tags['0008,0012'].keys()
assert str(instance) == f'Instance({an_instance.IDENTIFIER})'


def test_get_tag_content(instance):
Expand Down
1 change: 1 addition & 0 deletions tests/test_patient.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def test_attributes(patient):
assert patient.labels == [LABEL_PATIENT]
assert not patient.is_stable
assert isinstance(patient.last_update, datetime)
assert str(patient) == f'Patient({a_patient.IDENTIFIER})'

assert [s.identifier for s in patient.studies] == a_patient.INFORMATION['Studies']

Expand Down
1 change: 1 addition & 0 deletions tests/test_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def test_attributes(series):
assert not series.is_stable
assert isinstance(series.last_update, datetime)
assert series.instances != []
assert str(series) == f'Series({a_series.IDENTIFIER})'


def test_zip(series):
Expand Down
1 change: 1 addition & 0 deletions tests/test_study.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def test_attributes(study):
assert not study.is_stable
assert isinstance(study.last_update, datetime)
assert study.series != []
assert str(study) == f'Study({a_study.IDENTIFIER})'


def test_remove_empty_series(study):
Expand Down

0 comments on commit 4f118f9

Please sign in to comment.