Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lock state on query and __repr__ of resources #29

Merged
merged 1 commit into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading