Repository: pydicom/pydicom

Issue:
Allow to search a list of elements in a `FileSet` while only loading instances once, to drastically improve execution time
**Is your feature request related to a problem? Please describe.**
Currently, `fileset.FileSet.find_values` only allows for elements to be searched for one at a time. When executing this action while setting `load` to `True`, this results in a substantial overhead.

**Describe the solution you'd like**
The following example code allows loading the instances once, and iterating over a list of elements to find:
```python
def find_values_quick(self, elements, instances=None):
    results = {element: [] for element in elements}
    instances = instances or iter(self)
    for instance in instances:
        instance = instance.load()
        for element in elements:
            if element not in instance:
                continue
            val = instance[element].value
            if val not in results[element]:
                results[element].append(val)
    return results
```

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#1428](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (0132f87) into [master](https://codecov.io/gh/pydicom/pydicom/commit/699c9f0a8e190d463dd828822106250523d38154?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (699c9f0) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1428/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

```diff
@@           Coverage Diff           @@
##           master    #1428   +/-   ##
=======================================
  Coverage   95.59%   95.60%           
=======================================
  Files          65       65           
  Lines       10203    10210    +7     
=======================================
+ Hits         9754     9761    +7     
  Misses        449      449           
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Coverage Δ | |
|---|---|---|
| [pydicom/fileset.py](https://codecov.io/gh/pydicom/pydicom/pull/1428/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9maWxlc2V0LnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Last update [699c9f0...0132f87](https://codecov.io/gh/pydicom/pydicom/pull/1428?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom).

thomsentner: @scaramallion PR is nearly complete. I'm seeing an error about type checking of which I'm unsure how to resolve, don't have any experience with Python typing myself.
pep8speaks: Hello @thomsentner! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

* In the file [`pydicom/fileset.py`](https://github.com/pydicom/pydicom/blob/703e3af4d1df13aaf5505d57e2649d950cea4831/pydicom/fileset.py):

> [Line 1546:1](https://github.com/pydicom/pydicom/blob/703e3af4d1df13aaf5505d57e2649d950cea4831/pydicom/fileset.py#L1546): [W293](https://duckduckgo.com/?q=pep8%20W293) blank line contains whitespace





Patch:
diff --git a/pydicom/fileset.py b/pydicom/fileset.py
index d45bf9a45..cf728d40d 100644
--- a/pydicom/fileset.py
+++ b/pydicom/fileset.py
@@ -1486,16 +1486,16 @@ class FileSet:
 
     def find_values(
         self,
-        element: Union[str, int],
+        elements: Union[str, int, List[Union[str, int]]],
         instances: Optional[List[FileInstance]] = None,
         load: bool = False
-    ) -> List[Any]:
-        """Return a list of unique values for a given element.
+    ) -> Union[List[Any], Dict[Union[str, int], List]]:
+        """Return a list of unique values for given element(s).
 
         Parameters
         ----------
-        element : str, int or pydicom.tag.BaseTag
-            The keyword or tag of the element to search for.
+        elements : str, int or pydicom.tag.BaseTag, or list of these
+            The keyword or tag of the element(s) to search for.
         instances : list of pydicom.fileset.FileInstance, optional
             Search within the given instances. If not used then all available
             instances will be searched.
@@ -1508,32 +1508,41 @@ class FileSet:
 
         Returns
         -------
-        list of object
-            A list of value(s) for the element available in the instances.
+        list of object(s), or dict of lists of object(s)
+
+            * If single element was queried: A list of value(s) for the element
+              available in the instances.
+            * If list of elements was queried: A dict of element value pairs
+              with lists of value(s) for the elements available in the instances.
         """
-        has_element = False
-        results = []
+        element_list = elements if isinstance(elements, list) else [elements]
+        has_element = {element: False for element in element_list}
+        results: Dict = {element: [] for element in element_list}
         iter_instances = instances or iter(self)
         instance: Union[Dataset, FileInstance]
         for instance in iter_instances:
             if load:
                 instance = instance.load()
-
-            if element not in instance:
-                continue
-
-            has_element = True
-            val = instance[element].value
-            # Not very efficient, but we can't use set
-            if val not in results:
-                results.append(val)
-
-        if not load and not has_element:
+            for element in element_list:
+                if element not in instance:
+                    continue
+
+                has_element[element] = True
+                val = instance[element].value
+                # Not very efficient, but we can't use set
+                if val not in results[element]:
+                    results[element].append(val)
+        missing_elements = [
+            element for element, v in has_element.items() if not v
+        ]
+        if not load and missing_elements:
             warnings.warn(
                 "None of the records in the DICOMDIR dataset contain "
-                "the query element, consider using the 'load' parameter "
+                f"{missing_elements}, consider using the 'load' parameter "
                 "to expand the search to the corresponding SOP instances"
             )
+        if not isinstance(elements, list):
+            return results[element_list[0]]
 
         return results
 


Test patch:
diff --git a/pydicom/tests/test_fileset.py b/pydicom/tests/test_fileset.py
index 8ddc42260..12ccb7e9d 100644
--- a/pydicom/tests/test_fileset.py
+++ b/pydicom/tests/test_fileset.py
@@ -1945,33 +1945,45 @@ class TestFileSet_Load:
     def test_find_values(self, private):
         """Test searching the FileSet for element values."""
         fs = FileSet(private)
-        assert ['77654033', '98890234'] == fs.find_values("PatientID")
-        assert (
-            [
+        expected = {
+            "PatientID": ['77654033', '98890234'],
+            "StudyDescription": [
                 'XR C Spine Comp Min 4 Views',
                 'CT, HEAD/BRAIN WO CONTRAST',
                 '',
                 'Carotids',
                 'Brain',
-                'Brain-MRA'
-            ] == fs.find_values("StudyDescription")
-        )
+                'Brain-MRA',
+            ],
+        }
+        for k, v in expected.items():
+            assert fs.find_values(k) == v
+        assert fs.find_values(list(expected.keys())) == expected
 
     def test_find_values_load(self, private):
         """Test FileSet.find_values(load=True)."""
         fs = FileSet(private)
+        search_element = "PhotometricInterpretation"
         msg = (
             r"None of the records in the DICOMDIR dataset contain "
-            r"the query element, consider using the 'load' parameter "
+            fr"\['{search_element}'\], consider using the 'load' parameter "
             r"to expand the search to the corresponding SOP instances"
         )
         with pytest.warns(UserWarning, match=msg):
-            results = fs.find_values("PhotometricInterpretation", load=False)
+            results = fs.find_values(search_element, load=False)
             assert not results
 
-        assert ['MONOCHROME1', 'MONOCHROME2'] == fs.find_values(
-            "PhotometricInterpretation", load=True
-        )
+        assert fs.find_values(search_element, load=True) == [
+            'MONOCHROME1', 'MONOCHROME2'
+        ]
+
+        with pytest.warns(UserWarning, match=msg):
+            results = fs.find_values([search_element], load=False)
+            assert not results[search_element]
+
+        assert (
+            fs.find_values([search_element], load=True)
+        ) == {search_element: ['MONOCHROME1', 'MONOCHROME2']}
 
     def test_empty_file_id(self, dicomdir):
         """Test loading a record with an empty File ID."""