Repository: pydicom/pynetdicom
Pull request #837 (2023-06-16 15:23:12)

[MRG] Add UPS and BWM to qrscp and UPS to findscu
<!--
Please prefix your PR title with [WIP] for PRs that are in
progress and [MRG] when you consider them ready for review.
-->
#### Reference issue
#835 
#### Tasks
- [x] Unit tests added that reproduce issue or prove feature is working
- [x] Fix or feature added: Add UPS to findscu, Add minimal support for UPS and BWM to qrscp sufficient to allow testing of findscu
- [x] Documentation and examples updated (if relevant)
- [x] Unit tests passing and coverage at 100% after adding fix/feature (I think same coverage % as before)
- [x] Type annotations updated and passing with mypy (findscu has same issues as before my PR)
- [x] Apps updated and tested (if relevant)


sjswerdloff: When I use a lesser version of Sphinx, everything is fine in terms of building docs.
I thought I posted that in a previous comment.
When I try to update various dependencies that are listed in complaints by pip about latest Sphinx (7.0.1), they uninstall Spring 7 and install Sphinx 5.x

sjswerdloff: > When I use a lesser version of Sphinx, everything is fine in terms of building docs.
> I thought I posted that in a previous comment.
> When I try to update various dependencies that are listed in complaints by pip about latest Sphinx (7.0.1), they uninstall Spring 7 and install Sphinx 5.x

but now I get:
WARNING: extlinks: Sphinx-6.0 will require a caption string to contain exactly one '%s' and all other '%' need to be escaped as '%%'.

which is a bit more informative.
But I don't think I added anything that is any different from what was there before (I basically did copy/paste/modify)
darcymason: > I will wait some time if somebody else will have a look (@darcymason ?), and for the linter issue to be handled before merging this.

I've had a quick look without significant comments - will try to do a proper review in the next couple of days.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pydicom/pynetdicom/pull/837?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#837](https://app.codecov.io/gh/pydicom/pynetdicom/pull/837?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (1b87f23) into [master](https://app.codecov.io/gh/pydicom/pynetdicom/commit/78d4b95339daec8d83b0fcd9dc912e8653af61c7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (78d4b95) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.

```diff
@@             Coverage Diff             @@
##            master     #837      +/-   ##
===========================================
- Coverage   100.00%   99.97%   -0.03%     
===========================================
  Files           28       28              
  Lines         8639     8639              
===========================================
- Hits          8639     8637       -2     
- Misses           0        2       +2     
```

[see 1 file with indirect coverage changes](https://app.codecov.io/gh/pydicom/pynetdicom/pull/837/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

mrbean-bremen: The checks now pass, only waiting for the review by @darcymason.
mrbean-bremen: > If you want me to remove the minimal support for UPS CFIND from qrscp and will still approve the changes for findscu, I will create a new PR and cancel this one.

I'm fine with this - while I'm also not an expert in DICOM communication, I think it is a good addition - thank you!

Patch:
diff --git a/docs/apps/findscu.rst b/docs/apps/findscu.rst
index e2fcd411d..370835c2b 100644
--- a/docs/apps/findscu.rst
+++ b/docs/apps/findscu.rst
@@ -95,6 +95,8 @@ Query Information Model Options
             use patient/study only information model
 ``-W    --worklist``
             use modality worklist information model
+``-U    --ups``
+            use unified procedure step pull information model 
 
 Query Options
 -------------
@@ -185,6 +187,35 @@ SOP Classes
 +-----------------------------+-----------------------------------------------+
 
 
+Transfer Syntaxes
+.................
+
++------------------------+----------------------------------------------------+
+| UID                    | Transfer Syntax                                    |
++========================+====================================================+
+| 1.2.840.10008.1.2      | Implicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1    | Explicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1.99 | Deflated Explicit VR Little Endian                 |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.2    | Explicit VR Big Endian                             |
++------------------------+----------------------------------------------------+
+
+Unified Procedure Step  Service
+-------------------------------
+
+
+SOP Classes
+...........
+
++-----------------------------+-----------------------------------------------+
+| UID                         | Transfer Syntax                               |
++=============================+===============================================+
+| 1.2.840.10008.5.1.4.34.6.3  | UPS Pull Information Model - FIND             |
++-----------------------------+-----------------------------------------------+
+
+
 Transfer Syntaxes
 .................
 
diff --git a/docs/apps/qrscp.rst b/docs/apps/qrscp.rst
index 06072378b..23a4db713 100644
--- a/docs/apps/qrscp.rst
+++ b/docs/apps/qrscp.rst
@@ -27,6 +27,10 @@ SOP Instances sent to the application using the Storage service have some of
 their attributes added to a sqlite database that is used to manage Instances
 for the Query/Retrieve service.
 
+In addition, the ``qrscp`` application implements a Service Class Provider (SCP) for the
+:dcm:`Basic Modality Worklist<part04/chapter_K.html>`, and :dcm:`Unified Procedure Step<part04/Chapter_CC>`
+service classes, but currently will only return empty results (0 records)
+
 .. warning::
 
     In addition to the standard *pynetdicom* dependencies, the ``qrscp``
@@ -652,6 +656,63 @@ SOP Classes
 |                                  | Model - GET                              |
 +----------------------------------+------------------------------------------+
 
+Transfer Syntaxes
+.................
+
++------------------------+----------------------------------------------------+
+| UID                    | Transfer Syntax                                    |
++========================+====================================================+
+| 1.2.840.10008.1.2      | Implicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1    | Explicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1.99 | Deflated Explicit VR Little Endian                 |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.2    | Explicit VR Big Endian                             |
++------------------------+----------------------------------------------------+
+
+Basic Worklist Management Service
+---------------------------------
+
+SOP Classes
+...........
+
++-----------------------------+-----------------------------------------------+
+| UID                         | Transfer Syntax                               |
++=============================+===============================================+
+| 1.2.840.10008.5.1.4.31      | Modality Worklist Information Model - FIND    |
++-----------------------------+-----------------------------------------------+
+
+
+Transfer Syntaxes
+.................
+
++------------------------+----------------------------------------------------+
+| UID                    | Transfer Syntax                                    |
++========================+====================================================+
+| 1.2.840.10008.1.2      | Implicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1    | Explicit VR Little Endian                          |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.1.99 | Deflated Explicit VR Little Endian                 |
++------------------------+----------------------------------------------------+
+| 1.2.840.10008.1.2.2    | Explicit VR Big Endian                             |
++------------------------+----------------------------------------------------+
+
+Unified Procedure Step  Service
+-------------------------------
+
+
+SOP Classes
+...........
+
++-----------------------------+-----------------------------------------------+
+| UID                         | Transfer Syntax                               |
++=============================+===============================================+
+| 1.2.840.10008.5.1.4.34.6.3  | UPS Pull Information Model - FIND             |
++-----------------------------+-----------------------------------------------+
+
+
 Transfer Syntaxes
 .................
 
diff --git a/pynetdicom/apps/findscu/findscu.py b/pynetdicom/apps/findscu/findscu.py
index 1d1092624..c18d51f97 100755
--- a/pynetdicom/apps/findscu/findscu.py
+++ b/pynetdicom/apps/findscu/findscu.py
@@ -18,12 +18,14 @@ from pynetdicom import (
     PYNETDICOM_IMPLEMENTATION_UID,
     PYNETDICOM_IMPLEMENTATION_VERSION,
     PYNETDICOM_UID_PREFIX,
+    UnifiedProcedurePresentationContexts,
 )
 from pynetdicom.apps.common import create_dataset, setup_logging
 from pynetdicom._globals import DEFAULT_MAX_LENGTH
 from pynetdicom.pdu_primitives import SOPClassExtendedNegotiation
 from pynetdicom.sop_class import (
     ModalityWorklistInformationFind,
+    UnifiedProcedureStepPull,
     PatientRootQueryRetrieveInformationModelFind,
     StudyRootQueryRetrieveInformationModelFind,
     PatientStudyOnlyQueryRetrieveInformationModelFind,
@@ -173,6 +175,12 @@ def _setup_argparser():
         help="use modality worklist information model",
         action="store_true",
     )
+    qr_model.add_argument(
+        "-U",
+        "--ups",
+        help="use unified procedure step information model",
+        action="store_true",
+    )
 
     qr_query = parser.add_argument_group("Query Options")
     qr_query.add_argument(
@@ -303,12 +311,16 @@ def main(args=None):
 
     # Set the Presentation Contexts we are requesting the Find SCP support
     ae.requested_contexts = (
-        QueryRetrievePresentationContexts + BasicWorklistManagementPresentationContexts
+        QueryRetrievePresentationContexts
+        + BasicWorklistManagementPresentationContexts
+        + UnifiedProcedurePresentationContexts
     )
 
     # Query/Retrieve Information Models
     if args.worklist:
         query_model = ModalityWorklistInformationFind
+    elif args.ups:
+        query_model = UnifiedProcedureStepPull
     elif args.study:
         query_model = StudyRootQueryRetrieveInformationModelFind
     elif args.psonly:
diff --git a/pynetdicom/apps/qrscp/handlers.py b/pynetdicom/apps/qrscp/handlers.py
index f91288b12..5a32e58ab 100644
--- a/pynetdicom/apps/qrscp/handlers.py
+++ b/pynetdicom/apps/qrscp/handlers.py
@@ -62,43 +62,50 @@ def handle_find(event, db_path, cli_config, logger):
 
     model = event.request.AffectedSOPClassUID
 
-    engine = create_engine(db_path)
-    with engine.connect() as conn:
-        Session = sessionmaker(bind=engine)
-        session = Session()
-        # Search database using Identifier as the query
-        try:
-            matches = search(model, event.identifier, session)
-        except InvalidIdentifier as exc:
-            session.rollback()
-            logger.error("Invalid C-FIND Identifier received")
-            logger.error(str(exc))
-            yield 0xA900, None
-            return
-        except Exception as exc:
-            session.rollback()
-            logger.error("Exception occurred while querying database")
-            logger.exception(exc)
-            yield 0xC320, None
-            return
-        finally:
-            session.close()
-
-    # Yield results
-    for match in matches:
-        if event.is_cancelled:
-            yield 0xFE00, None
-            return
-
-        try:
-            response = match.as_identifier(event.identifier, model)
-            response.RetrieveAETitle = event.assoc.ae.ae_title
-        except Exception as exc:
-            logger.error("Error creating response Identifier")
-            logger.exception(exc)
-            yield 0xC322, None
-
-        yield 0xFF00, response
+    if model.keyword in (
+        "UnifiedProcedureStepPull",
+        "ModalityWorklistInformationModelFind",
+    ):
+        yield 0x0000, None
+    else:
+        engine = create_engine(db_path)
+        with engine.connect() as conn:
+            Session = sessionmaker(bind=engine)
+            session = Session()
+            # Search database using Identifier as the query
+            try:
+                matches = search(model, event.identifier, session)
+
+            except InvalidIdentifier as exc:
+                session.rollback()
+                logger.error("Invalid C-FIND Identifier received")
+                logger.error(str(exc))
+                yield 0xA900, None
+                return
+            except Exception as exc:
+                session.rollback()
+                logger.error("Exception occurred while querying database")
+                logger.exception(exc)
+                yield 0xC320, None
+                return
+            finally:
+                session.close()
+
+        # Yield results
+        for match in matches:
+            if event.is_cancelled:
+                yield 0xFE00, None
+                return
+
+            try:
+                response = match.as_identifier(event.identifier, model)
+                response.RetrieveAETitle = event.assoc.ae.ae_title
+            except Exception as exc:
+                logger.error("Error creating response Identifier")
+                logger.exception(exc)
+                yield 0xC322, None
+
+            yield 0xFF00, response
 
 
 def handle_get(event, db_path, cli_config, logger):
diff --git a/pynetdicom/apps/qrscp/qrscp.py b/pynetdicom/apps/qrscp/qrscp.py
index fc3fe070d..d380c4ec0 100755
--- a/pynetdicom/apps/qrscp/qrscp.py
+++ b/pynetdicom/apps/qrscp/qrscp.py
@@ -16,17 +16,20 @@ from pynetdicom import (
     evt,
     AllStoragePresentationContexts,
     ALL_TRANSFER_SYNTAXES,
+    UnifiedProcedurePresentationContexts,
 )
 from pynetdicom import _config, _handlers
 from pynetdicom.apps.common import setup_logging
 from pynetdicom.sop_class import (
     Verification,
+    ModalityWorklistInformationFind,
     PatientRootQueryRetrieveInformationModelFind,
     PatientRootQueryRetrieveInformationModelMove,
     PatientRootQueryRetrieveInformationModelGet,
     StudyRootQueryRetrieveInformationModelFind,
     StudyRootQueryRetrieveInformationModelMove,
     StudyRootQueryRetrieveInformationModelGet,
+    UnifiedProcedureStepPull,
 )
 from pynetdicom.utils import set_ae
 
@@ -57,7 +60,7 @@ _handlers._send_c_store_rq = _dont_log
 _handlers._recv_c_store_rsp = _dont_log
 
 
-__version__ = "1.0.1"
+__version__ = "1.1.0"
 
 
 def _log_config(config, logger):
@@ -386,6 +389,15 @@ def main(args=None):
     ae.add_supported_context(StudyRootQueryRetrieveInformationModelMove)
     ae.add_supported_context(StudyRootQueryRetrieveInformationModelGet)
 
+    # Unified Procedure Step SCP
+    for cx in UnifiedProcedurePresentationContexts:
+        ae.add_supported_context(
+            cx.abstract_syntax, ALL_TRANSFER_SYNTAXES, scp_role=True, scu_role=False
+        )
+
+    # Modality Worklist SCP
+    ae.add_supported_context(ModalityWorklistInformationFind)
+
     # Set our handler bindings
     handlers = [
         (evt.EVT_C_ECHO, handle_echo, [args, APP_LOGGER]),


Test patch:
diff --git a/pynetdicom/apps/tests/test_findscu.py b/pynetdicom/apps/tests/test_findscu.py
index ed19a5a0b..095253694 100644
--- a/pynetdicom/apps/tests/test_findscu.py
+++ b/pynetdicom/apps/tests/test_findscu.py
@@ -23,6 +23,7 @@ from pynetdicom import (
     DEFAULT_TRANSFER_SYNTAXES,
     QueryRetrievePresentationContexts,
     BasicWorklistManagementPresentationContexts,
+    UnifiedProcedurePresentationContexts,
 )
 from pynetdicom.sop_class import (
     Verification,
@@ -30,6 +31,7 @@ from pynetdicom.sop_class import (
     StudyRootQueryRetrieveInformationModelFind,
     PatientStudyOnlyQueryRetrieveInformationModelFind,
     ModalityWorklistInformationFind,
+    UnifiedProcedureStepPull,
 )
 
 
@@ -89,6 +91,7 @@ class FindSCUBase:
         ae.supported_contexts = (
             QueryRetrievePresentationContexts
             + BasicWorklistManagementPresentationContexts
+            + UnifiedProcedurePresentationContexts
         )
         scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers)
 
@@ -111,7 +114,7 @@ class FindSCUBase:
         assert {} == requestor.sop_class_extended
         assert requestor.user_identity == None
         cxs = requestor.primitive.presentation_context_definition_list
-        assert len(cxs) == 13
+        assert len(cxs) == 18
         cxs = {cx.abstract_syntax: cx for cx in cxs}
         assert PatientRootQueryRetrieveInformationModelFind in cxs
         cx = cxs[PatientRootQueryRetrieveInformationModelFind]
@@ -521,6 +524,39 @@ class FindSCUBase:
         cx = events[0].context
         assert cx.abstract_syntax == (PatientStudyOnlyQueryRetrieveInformationModelFind)
 
+    def test_flag_ups(self):
+        """Test the -U flag."""
+        events = []
+
+        def handle_find(event):
+            events.append(event)
+            yield 0x0000, None
+
+        handlers = [
+            (evt.EVT_C_FIND, handle_find),
+        ]
+
+        self.ae = ae = AE()
+        ae.acse_timeout = 5
+        ae.dimse_timeout = 5
+        ae.network_timeout = 5
+        ae.supported_contexts = (
+            QueryRetrievePresentationContexts
+            + BasicWorklistManagementPresentationContexts
+            + UnifiedProcedurePresentationContexts
+        )
+        scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers)
+
+        p = self.func(["-U", "-k", "PatientName="])
+        p.wait()
+        assert p.returncode == 0
+
+        scp.shutdown()
+
+        assert events[0].event == evt.EVT_C_FIND
+        cx = events[0].context
+        assert cx.abstract_syntax == UnifiedProcedureStepPull
+
     def test_flag_worklist(self):
         """Test the -W flag."""
         events = []
diff --git a/pynetdicom/apps/tests/test_storescu.py b/pynetdicom/apps/tests/test_storescu.py
index 1a32975ef..88b8cf8e6 100644
--- a/pynetdicom/apps/tests/test_storescu.py
+++ b/pynetdicom/apps/tests/test_storescu.py
@@ -119,7 +119,6 @@ class StoreSCUBase:
         p = self.func([DATASET_FILE])
         p.wait()
         assert p.returncode == 1
-
         out, err = capfd.readouterr()
         assert "Association request failed: unable to connect to remote" in err
         assert "TCP Initialisation Error" in err
@@ -566,7 +565,7 @@ class StoreSCUBase:
             ae.add_supported_context(cx.abstract_syntax, ALL_TRANSFER_SYNTAXES)
         scp = ae.start_server(("localhost", 11112), block=False, evt_handlers=handlers)
 
-        p = self.func([LIB_DIR, "--recurse", "-cx"])
+        p = self.func([DATA_DIR, "--recurse", "-cx"])
         p.wait()
         assert p.returncode == 0