Main dicom tags are only queried once, children only if lock=True

docs/tutorial/quickstart.md

@@ -134,8 +134,8 @@ The `pyorthanc.find()` function allow to find resources with filters on many lev
 or with complex filter. Each filter function takes an object that correspond to the resource level
 and should return a boolean value.
 
-Note that when using the `find()` function, the state of the resources `Patient/Study/Series/Instance` 
-are locked.
+Note that when using the `find()` function, the children of the resources `Patient/Study/Series/Instance`
+are only query once and then filtered accordingly to the provided filters.
 ```python
 from datetime import datetime
 from pyorthanc import find

pyorthanc/__init__.py

@@ -1,5 +1,5 @@
 from . import errors, util
-from ._filtering import build_patient_forest, find, trim_patients
+from ._filtering import find, trim_patients
 from ._find import find_instances, find_patients, find_series, find_studies, query_orthanc
 from ._modality import Modality, RemoteModality
 from ._resources import Instance, Patient, Series, Study
@@ -18,7 +18,6 @@
     'Study',
     'Series',
     'Instance',
-    'build_patient_forest',
     'trim_patients',
     'find',
     'find_patients',

pyorthanc/_filtering.py

@@ -55,7 +55,7 @@ def find(orthanc: Union[Orthanc, AsyncOrthanc],
             instance_filter=instance_filter
         ))
 
-    patients = [Patient(i, orthanc, lock=True) for i in orthanc.get_patients()]
+    patients = [Patient(i, orthanc, _lock_children=True) for i in orthanc.get_patients()]
     if patient_filter is not None:
         patients = [i for i in patients if patient_filter(i)]
 
@@ -109,7 +109,7 @@ async def _async_build_patient(
         study_filter: Optional[Callable],
         series_filter: Optional[Callable],
         instance_filter: Optional[Callable]) -> Patient:
-    patient = Patient(patient_id_, async_to_sync(async_orthanc), lock=True)
+    patient = Patient(patient_id_, async_to_sync(async_orthanc), _lock_children=True)
 
     if patient_filter is not None:
         if not patient_filter(patient):
@@ -136,7 +136,7 @@ async def _async_build_study(
         study_filter: Optional[Callable],
         series_filter: Optional[Callable],
         instance_filter: Optional[Callable]) -> Study:
-    study = Study(study_information['ID'], async_to_sync(async_orthanc), lock=True)
+    study = Study(study_information['ID'], async_to_sync(async_orthanc), _lock_children=True)
     study._information = study_information
 
     if study_filter is not None:
@@ -161,7 +161,7 @@ async def _async_build_series(
         async_orthanc: AsyncOrthanc,
         series_filter: Optional[Callable],
         instance_filter: Optional[Callable]) -> Series:
-    series = Series(series_information['ID'], async_to_sync(async_orthanc), lock=True)
+    series = Series(series_information['ID'], async_to_sync(async_orthanc), _lock_children=True)
     series._information = series_information
 
     if series_filter is not None:
@@ -181,7 +181,7 @@ def _build_instance(
         instance_information: Dict,
         orthanc: Orthanc,
         instance_filter: Optional[Callable]) -> Optional[Instance]:
-    instance = Instance(instance_information['ID'], orthanc, lock=True)
+    instance = Instance(instance_information['ID'], orthanc, _lock_children=True)
     instance._information = instance_information
 
     if instance_filter is not None:

pyorthanc/_find.py

@@ -196,7 +196,7 @@ def query_orthanc(client: Orthanc,
                   limit: int = DEFAULT_RESOURCES_LIMIT,
                   since: int = 0,
                   retrieve_all_resources: bool = True,
-                  lock: bool = False) -> List[Resource]:
+                  lock_children: bool = False) -> List[Resource]:
     """Query data in the Orthanc server
 
     Parameters
@@ -217,9 +217,10 @@ 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).
-
+    lock_children
+        If `lock_children` is True, the resource children (ex. instances of a series via `Series.instances`)
+        will be cached at the first query rather than queried every time. This is useful when you want
+        to filter the children of a resource and want to maintain the filter result.
     Returns
     -------
     List[Resource]
@@ -275,22 +276,16 @@ def query_orthanc(client: Orthanc,
         results = client.post_tools_find(data)
 
     if level == 'Patient':
-        resources = [Patient(i['ID'], client, lock=lock) for i in results]
+        resources = [Patient(i['ID'], client, _lock_children=lock_children) for i in results]
     elif level == 'Study':
-        resources = [Study(i['ID'], client, lock=lock) for i in results]
+        resources = [Study(i['ID'], client, _lock_children=lock_children) for i in results]
     elif level == 'Series':
-        resources = [Series(i['ID'], client, lock=lock) for i in results]
+        resources = [Series(i['ID'], client, _lock_children=lock_children) for i in results]
     elif level == 'Instance':
-        resources = [Instance(i['ID'], client, lock=lock) for i in results]
+        resources = [Instance(i['ID'], client, _lock_children=lock_children) for i in results]
     else:
         raise ValueError(f"Unknown level ['Patient', 'Study', 'Series', 'Instance'], got {level}")
 
-    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()
-
     return resources
 
 

pyorthanc/_resources/instance.py

@@ -85,13 +85,6 @@ def get_main_information(self) -> Dict:
         Dict
             Dictionary with tags as key and information as value
         """
-        if self.lock:
-            if self._information is None:
-                # Setup self._information for the first time when study is lock
-                self._information = self.client.get_instances_id(self.id_)
-
-            return self._information
-
         return self.client.get_instances_id(self.id_)
 
     @property
@@ -119,8 +112,8 @@ def creation_date(self) -> datetime:
         datetime
             Creation Date
         """
-        date_string = self.get_main_information()['MainDicomTags']['InstanceCreationDate']
-        time_string = self.get_main_information()['MainDicomTags']['InstanceCreationTime']
+        date_string = self._get_main_dicom_tag_value('InstanceCreationDate')
+        time_string = self._get_main_dicom_tag_value('InstanceCreationTime')
 
         return util.make_datetime_from_dicom_date(date_string, time_string)
 

pyorthanc/_resources/patient.py

@@ -25,13 +25,6 @@ def get_main_information(self) -> Dict:
         Dict
             Dictionary of patient main information.
         """
-        if self.lock:
-            if self._information is None:
-                # Setup self._information for the first time when patient is lock
-                self._information = self.client.get_patients_id(self.id_)
-
-            return self._information
-
         return self.client.get_patients_id(self.id_)
 
     @property
@@ -240,16 +233,16 @@ def studies(self) -> List[Study]:
         List[Study]
             List of the patient's studies
         """
-        if self.lock:
+        if self._lock_children:
             if self._child_resources is None:
                 studies_ids = self.get_main_information()['Studies']
-                self._child_resources = [Study(i, self.client, self.lock) for i in studies_ids]
+                self._child_resources = [Study(i, self.client, self._lock_children) for i in studies_ids]
 
             return self._child_resources
 
         studies_ids = self.get_main_information()['Studies']
 
-        return [Study(i, self.client, self.lock) for i in studies_ids]
+        return [Study(i, self.client) for i in studies_ids]
 
     def anonymize(self, remove: List = None, replace: Dict = None, keep: List = None,
                   force: bool = False, keep_private_tags: bool = False,
@@ -501,6 +494,9 @@ def modify(self, remove: List = None, replace: Dict = None, keep: List = None,
                 'Use `.modify_as_job` or increase client.timeout.'
             )
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         # if 'PatientID' is not affected, the modified_patient['ID'] is the same as self.id_
         return Patient(modified_patient['ID'], self.client)
 
@@ -593,6 +589,9 @@ def modify_as_job(self, remove: List = None, replace: Dict = None, keep: List =
 
         job_info = self.client.post_patients_id_modify(self.id_, data)
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         return Job(job_info['ID'], self.client)
 
     def get_shared_tags(self, simplify: bool = False, short: bool = False) -> Dict:

pyorthanc/_resources/resource.py

@@ -9,7 +9,7 @@
 
 class Resource:
 
-    def __init__(self, id_: str, client: Orthanc, lock: bool = False) -> None:
+    def __init__(self, id_: str, client: Orthanc, _lock_children: bool = False) -> None:
         """Constructor
 
         Parameters
@@ -18,19 +18,18 @@ def __init__(self, id_: str, client: Orthanc, lock: bool = False) -> None:
             Orthanc identifier of the resource
         client
             Orthanc client
-        lock
-            Specify if the Resource state should be locked. This is useful when the child resources
-            have been filtered out, and you don't want the resource to return an updated version
-            or all of those children. "lock=True" is notably used for the "find" function,
-            so that only the filtered resources are kept.
+        _lock_children
+            If `_lock_children` is True, the resource children (ex. instances of a series via `Series.instances`)
+            will be cached at the first query rather than queried every time. This is useful when you want
+            to filter the children of a resource and want to maintain the filter result.
         """
         client = util.ensure_non_raw_response(client)
 
         self.id_ = id_
         self.client = client
 
-        self.lock = lock
-        self._information: Optional[Dict] = None
+        self._lock_children = _lock_children
+        self._main_dicom_tags: Optional[Dict] = None
         self._child_resources: Optional[List['Resource']] = None
 
     @property
@@ -46,15 +45,18 @@ def identifier(self) -> str:
 
     @property
     def main_dicom_tags(self) -> Dict[str, str]:
-        return self.get_main_information()['MainDicomTags']
+        if self._main_dicom_tags is None:
+            self._main_dicom_tags = self.get_main_information()['MainDicomTags']
+
+        return self._main_dicom_tags
 
     @abc.abstractmethod
     def get_main_information(self):
         raise NotImplementedError
 
     def _get_main_dicom_tag_value(self, tag: str) -> Any:
         try:
-            return self.get_main_information()['MainDicomTags'][tag]
+            return self.main_dicom_tags[tag]
         except KeyError:
             raise errors.TagDoesNotExistError(f'{self} has no {tag} tag.')
 

pyorthanc/_resources/series.py

@@ -24,31 +24,30 @@ class Series(Resource):
     @property
     def instances(self) -> List[Instance]:
         """Get series instance"""
-        if self.lock:
+        if self._lock_children:
             if self._child_resources is None:
                 instances_ids = self.get_main_information()['Instances']
-                self._child_resources = [Instance(i, self.client, self.lock) for i in instances_ids]
+                self._child_resources = [Instance(i, self.client, self._lock_children) for i in instances_ids]
 
             return self._child_resources
 
         instances_ids = self.get_main_information()['Instances']
 
-        return [Instance(i, self.client, self.lock) for i in instances_ids]
+        return [Instance(i, self.client) for i in instances_ids]
 
     @property
     def uid(self) -> str:
         """Get SeriesInstanceUID"""
         return self._get_main_dicom_tag_value('SeriesInstanceUID')
 
     def get_main_information(self) -> Dict:
-        """Get series main information"""
-        if self.lock:
-            if self._information is None:
-                # Setup self._information for the first time when series is lock
-                self._information = self.client.get_series_id(self.id_)
-
-            return self._information
+        """Get series main information
 
+        Returns
+        -------
+        Dict
+            Dictionary of series information
+        """
         return self.client.get_series_id(self.id_)
 
     @property
@@ -435,6 +434,9 @@ def modify(self, remove: List = None, replace: Dict = None, keep: List = None,
                 'Use `.modify_as_job` or increase client.timeout.'
             )
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         # if 'SeriesInstanceUID' is not affected, the modified_series['ID'] is the same as self.id_
         return Series(modified_series['ID'], self.client)
 
@@ -542,6 +544,9 @@ def modify_as_job(self, remove: List = None, replace: Dict = None, keep: List =
 
         job_info = self.client.post_series_id_modify(self.id_, data)
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         return Job(job_info['ID'], self.client)
 
     def get_zip(self) -> bytes:

pyorthanc/_resources/study.py

@@ -29,13 +29,6 @@ def get_main_information(self) -> Dict:
         Dict
             Dictionary of study information
         """
-        if self.lock:
-            if self._information is None:
-                # Setup self._information for the first time when study is lock
-                self._information = self.client.get_studies_id(self.id_)
-
-            return self._information
-
         return self.client.get_studies_id(self.id_)
 
     @property
@@ -95,16 +88,16 @@ def patient_information(self) -> Dict:
     @property
     def series(self) -> List[Series]:
         """Get Study series"""
-        if self.lock:
+        if self._lock_children:
             if self._child_resources is None:
                 series_ids = self.get_main_information()['Series']
-                self._child_resources = [Series(i, self.client, self.lock) for i in series_ids]
+                self._child_resources = [Series(i, self.client, self._lock_children) for i in series_ids]
 
             return self._child_resources
 
         series_ids = self.get_main_information()['Series']
 
-        return [Series(i, self.client, self.lock) for i in series_ids]
+        return [Series(i, self.client) for i in series_ids]
 
     @property
     def accession_number(self) -> str:
@@ -393,6 +386,9 @@ def modify(self, remove: List = None, replace: Dict = None, keep: List = None,
                 'Use `.modify_as_job` or increase client.timeout.'
             )
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         # if 'StudyInstanceUID' is not affected, the modified_study['ID'] is the same as self.id_
         return Study(modified_study['ID'], self.client)
 
@@ -500,6 +496,9 @@ def modify_as_job(self, remove: List = None, replace: Dict = None, keep: List =
 
         job_info = self.client.post_studies_id_modify(self.id_, data)
 
+        # Reset cache since a main DICOM tag may have be changed
+        self._main_dicom_tags = None
+
         return Job(job_info['ID'], self.client)
 
     def get_zip(self) -> bytes:

tests/resources/test_patient.py

@@ -141,7 +141,6 @@ def test_modify_replace(patient):
 
 def test_modify_as_job_remove(patient):
     job = patient.modify_as_job(remove=['PatientName'])
-    assert patient.name == a_patient.NAME
 
     job.wait_until_completion()
     assert patient.patient_id == a_patient.ID
@@ -158,7 +157,6 @@ def test_modify_as_job_remove(patient):
 
 def test_modify_as_job_replace(patient: Patient):
     job = patient.modify_as_job(replace={'PatientName': 'NewName'})
-    assert patient.name == a_patient.NAME
 
     job.wait_until_completion()
     assert patient.patient_id == a_patient.ID