From 01f7d08ffa7e310c900c79145c53adf0c844366c Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Mon, 28 Apr 2025 18:21:01 +0200 Subject: [PATCH 1/8] adding optional params to search and retrieve functions --- src/dicomweb_client/web.py | 58 ++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index 6876ec4..8a24e38 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -1628,7 +1628,8 @@ def search_for_studies( offset: Optional[int] = None, fields: Optional[Sequence[str]] = None, search_filters: Optional[Dict[str, Any]] = None, - get_remaining: bool = False + get_remaining: bool = False, + additional_params: Optional[Dict[str, Any]] = None ) -> List[Dict[str, dict]]: """Search for studies. @@ -1649,6 +1650,8 @@ def search_for_studies( get_remaining: bool, optional Whether remaining results should be included (this may repeatedly query the server for remaining results) + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters to include in the request. Returns ------- @@ -1656,12 +1659,14 @@ def search_for_studies( Study representations (see `Study Result Attributes `_) - Note + Notes ---- - The server may only return a subset of search results. In this case, + - The server may only return a subset of search results. In this case, a warning will notify the client that there are remaining results. Remaining results can be requested via repeated calls using the `offset` parameter. + - If `additional_params` is provided, it will be merged into the standard query parameters, + with its values overwriting any existing keys if duplicates are present. """ # noqa: E501: E501 logger.info('search for studies') @@ -1673,6 +1678,8 @@ def search_for_studies( fields=fields, search_filters=search_filters ) + if additional_params: + params.update(additional_params) return self._http_get_application_json( url, params=params, @@ -1918,7 +1925,8 @@ def _get_study( self, study_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - stream: bool = False + stream: bool = False, + params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Get all instances of a study. @@ -1932,6 +1940,8 @@ def _get_study( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) + params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -1947,6 +1957,7 @@ def _get_study( if media_types is None: return self._http_get_multipart_application_dicom( url, + params=params, stream=stream ) common_media_types = self._get_common_media_types(media_types) @@ -1964,6 +1975,7 @@ def _get_study( return self._http_get_multipart_application_dicom( url, media_types=media_types, + params=params, stream=stream ) @@ -2129,7 +2141,8 @@ def search_for_series( offset: Optional[int] = None, fields: Optional[Sequence[str]] = None, search_filters: Optional[Dict[str, Any]] = None, - get_remaining: bool = False + get_remaining: bool = False, + additional_params: Optional[Dict[str, Any]] = None ) -> List[Dict[str, dict]]: """Search for series. @@ -2152,6 +2165,8 @@ def search_for_series( get_remaining: bool, optional Whether remaining results should be included (this may repeatedly query the server for remaining results) + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters to include in the request. Returns ------- @@ -2159,12 +2174,14 @@ def search_for_series( Series representations (see `Series Result Attributes `_) - Note + Notes ---- - The server may only return a subset of search results. In this case, + - The server may only return a subset of search results. In this case, a warning will notify the client that there are remaining results. Remaining results can be requested via repeated calls using the `offset` parameter. + - If `additional_params` is provided, it will be merged into the standard query parameters, + with its values overwriting any existing keys if duplicates are present. """ # noqa: E501 if study_instance_uid is not None: @@ -2180,6 +2197,8 @@ def search_for_series( fields=fields, search_filters=search_filters ) + if additional_params: + params.update(additional_params) return self._http_get_application_json( url, params=params, @@ -2191,7 +2210,8 @@ def _get_series( study_instance_uid: str, series_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - stream: bool = False + stream: bool = False, + params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Get instances of a series. @@ -2207,6 +2227,8 @@ def _get_series( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) + params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2236,6 +2258,7 @@ def _get_series( if media_types is None: return self._http_get_multipart_application_dicom( url, + params=params, stream=stream ) common_media_types = self._get_common_media_types(media_types) @@ -2253,6 +2276,7 @@ def _get_series( return self._http_get_multipart_application_dicom( url, media_types=media_types, + params=params, stream=stream ) @@ -2514,7 +2538,8 @@ def search_for_instances( offset: Optional[int] = None, fields: Optional[Sequence[str]] = None, search_filters: Optional[Dict[str, Any]] = None, - get_remaining: bool = False + get_remaining: bool = False, + additional_params: Optional[Dict[str, Any]] = None ) -> List[Dict[str, dict]]: """Search for instances. @@ -2539,6 +2564,8 @@ def search_for_instances( get_remaining: bool, optional Whether remaining results should be included (this may repeatedly query the server for remaining results) + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters to include in the request. Returns ------- @@ -2548,10 +2575,12 @@ def search_for_instances( Note ---- - The server may only return a subset of search results. In this case, + - The server may only return a subset of search results. In this case, a warning will notify the client that there are remaining results. Remaining results can be requested via repeated calls using the `offset` parameter. + - If `additional_params` is provided, it will be merged into the standard query parameters, + with its values overwriting any existing keys if duplicates are present. """ # noqa: E501 message = 'search for instances' @@ -2585,6 +2614,7 @@ def retrieve_instance( series_instance_uid: str, sop_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + params: Optional[Dict[str, Any]] = None ) -> pydicom.dataset.Dataset: """Retrieve an individual instance. @@ -2599,6 +2629,8 @@ def retrieve_instance( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes + params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2657,7 +2689,11 @@ def retrieve_instance( f'Media type "{common_media_type}" is not supported for ' 'retrieval of an instance. It must be "application/dicom".' ) - iterator = self._http_get_multipart_application_dicom(url, media_types) + iterator = self._http_get_multipart_application_dicom( + url, + media_types=media_types, + params=params + ) instances = list(iterator) if len(instances) > 1: # This should not occur, but safety first. From 3d55233aa699c203b1789bfcb07516fcc547c37b Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Tue, 29 Apr 2025 14:31:45 +0200 Subject: [PATCH 2/8] Added tests and additional params to more methods --- src/dicomweb_client/web.py | 117 +++++++++++++++++++++++++---------- tests/test_web.py | 123 +++++++++++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 32 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index 8a24e38..8fbf353 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -1802,6 +1802,7 @@ def _get_bulkdata( media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, byte_range: Optional[Tuple[int, int]] = None, stream: bool = False, + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[bytes]: """Get bulk data items at a given location. @@ -1817,6 +1818,8 @@ def _get_bulkdata( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -1826,26 +1829,45 @@ def _get_bulkdata( """ # noqa: E501 if media_types is None: return self._http_get_multipart( - url, media_types, byte_range=byte_range, stream=stream + url, + media_types, + byte_range=byte_range, + params=additional_params, + stream=stream, ) common_media_types = self._get_common_media_types(media_types) if len(common_media_types) > 1: return self._http_get_multipart( - url, media_types, byte_range=byte_range, stream=stream + url, media_types, + byte_range=byte_range, + params=additional_params, + stream=stream, ) else: common_media_type = common_media_types[0] if common_media_type == 'application/octet-stream': return self._http_get_multipart_application_octet_stream( - url, media_types, byte_range=byte_range, stream=stream + url, + media_types, + byte_range=byte_range, + params=additional_params, + stream=stream ) elif common_media_type.startswith('image'): return self._http_get_multipart_image( - url, media_types, byte_range=byte_range, stream=stream + url, + media_types, + byte_range=byte_range, + params=additional_params, + stream=stream ) elif common_media_type.startswith('video'): return self._http_get_multipart_video( - url, media_types, byte_range=byte_range, stream=stream + url, + media_types, + byte_range=byte_range, + params=additional_params, + stream=stream ) else: raise ValueError( @@ -1857,7 +1879,8 @@ def retrieve_bulkdata( self, url: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - byte_range: Optional[Tuple[int, int]] = None + byte_range: Optional[Tuple[int, int]] = None, + additional_params: Optional[Dict[str, Any]] = None, ) -> List[bytes]: """Retrieve bulk data at a given location. @@ -1870,6 +1893,8 @@ def retrieve_bulkdata( corresponding transfer syntaxes byte_range: Union[Tuple[int, int], None], optional Start and end of byte range + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -1882,7 +1907,8 @@ def retrieve_bulkdata( url=url, media_types=media_types, byte_range=byte_range, - stream=False + stream=False, + additional_params=additional_params, ) ) @@ -1890,7 +1916,8 @@ def iter_bulkdata( self, url: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - byte_range: Optional[Tuple[int, int]] = None + byte_range: Optional[Tuple[int, int]] = None, + additional_params: Optional[Dict[str, Any]] = None, ) -> Iterator[bytes]: """Iterate over bulk data items at a given location. @@ -1903,6 +1930,8 @@ def iter_bulkdata( corresponding transfer syntaxes byte_range: Union[Tuple[int, int], None], optional Start and end of byte range + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -1918,7 +1947,8 @@ def iter_bulkdata( url=url, media_types=media_types, byte_range=byte_range, - stream=True + stream=True, + additional_params=additional_params, ) def _get_study( @@ -1926,7 +1956,7 @@ def _get_study( study_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, stream: bool = False, - params: Optional[Dict[str, Any]] = None + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Get all instances of a study. @@ -1940,7 +1970,7 @@ def _get_study( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) - params: Union[Dict[str, Any], None], optional + additional_params: Union[Dict[str, Any], None], optional Additional HTTP GET query parameters Returns @@ -1957,7 +1987,7 @@ def _get_study( if media_types is None: return self._http_get_multipart_application_dicom( url, - params=params, + params=additional_params, stream=stream ) common_media_types = self._get_common_media_types(media_types) @@ -1975,7 +2005,7 @@ def _get_study( return self._http_get_multipart_application_dicom( url, media_types=media_types, - params=params, + params=additional_params, stream=stream ) @@ -2061,7 +2091,8 @@ def iter_study( def retrieve_study_metadata( self, - study_instance_uid: str + study_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None ) -> List[Dict[str, dict]]: """Retrieve metadata of all instances of a study. @@ -2069,6 +2100,8 @@ def retrieve_study_metadata( ---------- study_instance_uid: str Study Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2083,7 +2116,7 @@ def retrieve_study_metadata( ) url = self._get_studies_url(_Transaction.RETRIEVE, study_instance_uid) url += '/metadata' - return self._http_get_application_json(url) + return self._http_get_application_json(url, params=additional_params) def delete_study(self, study_instance_uid: str) -> None: """Delete all instances of a study. @@ -2211,7 +2244,7 @@ def _get_series( series_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, stream: bool = False, - params: Optional[Dict[str, Any]] = None + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Get instances of a series. @@ -2227,7 +2260,7 @@ def _get_series( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) - params: Union[Dict[str, Any], None], optional + additional_params: Union[Dict[str, Any], None], optional Additional HTTP GET query parameters Returns @@ -2258,7 +2291,7 @@ def _get_series( if media_types is None: return self._http_get_multipart_application_dicom( url, - params=params, + params=additional_params, stream=stream ) common_media_types = self._get_common_media_types(media_types) @@ -2276,7 +2309,7 @@ def _get_series( return self._http_get_multipart_application_dicom( url, media_types=media_types, - params=params, + params=additional_params, stream=stream ) @@ -2372,6 +2405,7 @@ def retrieve_series_metadata( self, study_instance_uid: str, series_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None ) -> List[Dict[str, dict]]: """Retrieve metadata for all instances of a series. @@ -2381,6 +2415,8 @@ def retrieve_series_metadata( Study Instance UID series_instance_uid: str Series Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2410,7 +2446,7 @@ def retrieve_series_metadata( series_instance_uid ) url += '/metadata' - return self._http_get_application_json(url) + return self._http_get_application_json(url, params=additional_params) def retrieve_series_rendered( self, study_instance_uid, @@ -2602,6 +2638,8 @@ def search_for_instances( fields=fields, search_filters=search_filters ) + if additional_params: + params.update(additional_params) return self._http_get_application_json( url, params=params, @@ -2614,7 +2652,7 @@ def retrieve_instance( series_instance_uid: str, sop_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - params: Optional[Dict[str, Any]] = None + additional_params: Optional[Dict[str, Any]] = None ) -> pydicom.dataset.Dataset: """Retrieve an individual instance. @@ -2629,7 +2667,7 @@ def retrieve_instance( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes - params: Union[Dict[str, Any], None], optional + additional_params: Union[Dict[str, Any], None], optional Additional HTTP GET query parameters Returns @@ -2692,7 +2730,7 @@ def retrieve_instance( iterator = self._http_get_multipart_application_dicom( url, media_types=media_types, - params=params + params=additional_params ) instances = list(iterator) if len(instances) > 1: @@ -2789,7 +2827,8 @@ def retrieve_instance_metadata( self, study_instance_uid: str, series_instance_uid: str, - sop_instance_uid: str + sop_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None ) -> Dict[str, dict]: """Retrieve metadata of an individual instance. @@ -2801,6 +2840,8 @@ def retrieve_instance_metadata( Series Instance UID sop_instance_uid: str SOP Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2830,7 +2871,7 @@ def retrieve_instance_metadata( sop_instance_uid ) url += '/metadata' - return self._http_get_application_json(url)[0] + return self._http_get_application_json(url, params=additional_params)[0] def retrieve_instance_rendered( self, @@ -2920,7 +2961,8 @@ def _get_instance_frames( sop_instance_uid: str, frame_numbers: Sequence[int], media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, - stream: bool = False + stream: bool = False, + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[bytes]: """Get frames of an instance. @@ -2940,6 +2982,8 @@ def _get_instance_frames( stream: bool, optional Whether data should be streamed (i.e., requested using chunked transfer encoding) + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2968,14 +3012,19 @@ def _get_instance_frames( frame_list = ','.join([str(n) for n in frame_numbers]) url += f'/frames/{frame_list}' if media_types is None: - return self._http_get_multipart(url, stream=stream) + return self._http_get_multipart( + url, + stream=stream, + params=additional_params + ) common_media_types = self._get_common_media_types(media_types) if len(common_media_types) > 1: return self._http_get_multipart( url, media_types=media_types, - stream=stream + stream=stream, + params=additional_params ) common_media_type = common_media_types[0] @@ -2983,25 +3032,29 @@ def _get_instance_frames( return self._http_get_multipart_application_octet_stream( url, media_types=media_types, - stream=stream + stream=stream, + params=additional_params ) elif common_media_type.startswith('image'): return self._http_get_multipart_image( url, media_types=media_types, - stream=stream + stream=stream, + params=additional_params ) elif common_media_type.startswith('video'): return self._http_get_multipart_video( url, media_types=media_types, - stream=stream + stream=stream, + params=additional_params ) elif common_media_type.startswith('*'): return self._http_get_multipart( url, media_types=media_types, - stream=stream + stream=stream, + params=additional_params ) else: raise ValueError( diff --git a/tests/test_web.py b/tests/test_web.py index ff5c866..35fe166 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1,4 +1,5 @@ import json +from operator import add import xml.etree.ElementTree as ET from io import BytesIO from http import HTTPStatus @@ -136,6 +137,24 @@ def test_search_for_studies(httpserver, client, cache_dir): ) +def test_search_for_studies_with_additional_params(httpserver, client, cache_dir): + cache_filename = str(cache_dir.joinpath('search_for_studies.json')) + with open(cache_filename, 'r') as f: + content = f.read() + parsed_content = json.loads(content) + headers = {'content-type': 'application/dicom+json'} + httpserver.serve_content(content=content, code=200, headers=headers) + params = {"key1" : ["value1", "value2"], "key2" : "value3"} + assert client.search_for_studies(additional_params=params) == parsed_content + request = httpserver.requests[0] + assert request.path == '/studies' + assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert all( + mime[0] in ('application/json', 'application/dicom+json') + for mime in request.accept_mimetypes + ) + + def test_search_for_studies_with_retries(httpserver, client, cache_dir): headers = {'content-type': 'application/dicom+json'} max_attempts = 3 @@ -217,6 +236,24 @@ def test_search_for_series(httpserver, client, cache_dir): ) +def test_search_for_series_with_additional_params(httpserver, client, cache_dir): + cache_filename = str(cache_dir.joinpath('search_for_series.json')) + with open(cache_filename, 'r') as f: + content = f.read() + parsed_content = json.loads(content) + headers = {'content-type': 'application/dicom+json'} + httpserver.serve_content(content=content, code=200, headers=headers) + params = {"key1" : ["value1", "value2"], "key2" : "value3"} + assert client.search_for_series(additional_params=params) == parsed_content + request = httpserver.requests[0] + assert request.path == '/series' + assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert all( + mime[0] in ('application/json', 'application/dicom+json') + for mime in request.accept_mimetypes + ) + + def test_search_for_series_filter_modality(httpserver, client, cache_dir): cache_filename = str(cache_dir.joinpath('search_for_series.json')) with open(cache_filename, 'r') as f: @@ -288,6 +325,24 @@ def test_search_for_instances(httpserver, client, cache_dir): ) +def test_search_for_instances_with_additional_params(httpserver, client, cache_dir): + cache_filename = str(cache_dir.joinpath('search_for_instances.json')) + with open(cache_filename, 'r') as f: + content = f.read() + parsed_content = json.loads(content) + headers = {'content-type': 'application/dicom+json'} + httpserver.serve_content(content=content, code=200, headers=headers) + params = {"key1" : ["value1", "value2"], "key2" : "value3"} + assert client.search_for_instances(additional_params=params) == parsed_content + request = httpserver.requests[0] + assert request.path == '/instances' + assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert all( + mime[0] in ('application/json', 'application/dicom+json') + for mime in request.accept_mimetypes + ) + + def test_search_for_instances_of_series(httpserver, client, cache_dir): cache_filename = str(cache_dir.joinpath('search_for_instances.json')) with open(cache_filename, 'r') as f: @@ -380,6 +435,35 @@ def test_retrieve_instance_metadata(httpserver, client, cache_dir): ) +def test_retrieve_instance_metadata_with_additional_params(httpserver, client, cache_dir): + cache_filename = str(cache_dir.joinpath('retrieve_instance_metadata.json')) + with open(cache_filename, 'r') as f: + content = f.read() + parsed_content = json.loads(content) + headers = {'content-type': 'application/dicom+json'} + httpserver.serve_content(content=content, code=200, headers=headers) + params = {"key1" : ["value1", "value2"], "key2" : "value3"} + study_instance_uid = '1.2.3' + series_instance_uid = '1.2.4' + sop_instance_uid = '1.2.5' + result = client.retrieve_instance_metadata( + study_instance_uid, series_instance_uid, sop_instance_uid, additional_params=params + ) + assert result == parsed_content[0] + request = httpserver.requests[0] + assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + expected_path = ( + f'/studies/{study_instance_uid}' + f'/series/{series_instance_uid}' + f'/instances/{sop_instance_uid}/metadata' + ) + assert request.path == expected_path + assert all( + mime[0] in ('application/json', 'application/dicom+json') + for mime in request.accept_mimetypes + ) + + def test_retrieve_instance_metadata_wado_prefix(httpserver, client, cache_dir): client.wado_url_prefix = 'wadors' cache_filename = str(cache_dir.joinpath('retrieve_instance_metadata.json')) @@ -519,6 +603,45 @@ def test_retrieve_instance(httpserver, client, cache_dir): assert request.path == expected_path assert request.accept_mimetypes[0][0][:43] == headers['content-type'][:43] +def test_retrieve_instance_with_additional_params(httpserver, client, cache_dir): + cache_filename = str(cache_dir.joinpath('file.dcm')) + with open(cache_filename, 'rb') as f: + data = f.read() + media_type = 'application/dicom' + boundary = 'boundary' + headers = { + 'content-type': ( + 'multipart/related; ' + f'type="{media_type}"; ' + f'boundary="{boundary}"' + ), + } + message = DICOMwebClient._encode_multipart_message( + content=[data], + content_type=headers['content-type'] + ) + httpserver.serve_content(content=message, code=200, headers=headers) + params = {"key1" : ["value1", "value2"], "key2" : "value3"} + study_instance_uid = '1.2.3' + series_instance_uid = '1.2.4' + sop_instance_uid = '1.2.5' + response = client.retrieve_instance( + study_instance_uid, series_instance_uid, sop_instance_uid, additional_params=params + ) + with BytesIO() as fp: + pydicom.dcmwrite(fp, response) + raw_result = fp.getvalue() + assert raw_result == data + request = httpserver.requests[0] + assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + expected_path = ( + f'/studies/{study_instance_uid}' + f'/series/{series_instance_uid}' + f'/instances/{sop_instance_uid}' + ) + assert request.path == expected_path + assert request.accept_mimetypes[0][0][:43] == headers['content-type'][:43] + def test_retrieve_instance_singlepart(httpserver, client, cache_dir): cache_filename = str(cache_dir.joinpath('file.dcm')) From ff10780c9394cbd1ebaa455920633e8beafc2c75 Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Tue, 29 Apr 2025 15:19:39 +0200 Subject: [PATCH 3/8] adding params to retrieve_instance_frames --- src/dicomweb_client/web.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index 8fbf353..a773909 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -3068,7 +3068,8 @@ def retrieve_instance_frames( series_instance_uid: str, sop_instance_uid: str, frame_numbers: Sequence[int], - media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None + media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> List[bytes]: """Retrieve one or more frames of an image instance. @@ -3085,6 +3086,8 @@ def retrieve_instance_frames( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the corresponding transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -3099,7 +3102,8 @@ def retrieve_instance_frames( sop_instance_uid=sop_instance_uid, frame_numbers=frame_numbers, media_types=media_types, - stream=False + stream=False, + additional_params=additional_params ) ) @@ -3109,7 +3113,8 @@ def iter_instance_frames( series_instance_uid: str, sop_instance_uid: str, frame_numbers: Sequence[int], - media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None + media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[bytes]: """Iterate over frames of an image instance. @@ -3126,6 +3131,8 @@ def iter_instance_frames( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the corresponding transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -3143,7 +3150,8 @@ def iter_instance_frames( sop_instance_uid=sop_instance_uid, frame_numbers=frame_numbers, media_types=media_types, - stream=True + stream=True, + additional_params=additional_params ) def retrieve_instance_frames_rendered( From 315ff4168911945c5f21b295f6feaec05ed44f93 Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Wed, 30 Apr 2025 13:37:14 +0200 Subject: [PATCH 4/8] fixing broken tests --- src/dicomweb_client/web.py | 6 +-- tests/test_web.py | 76 ++++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index a773909..171f3fc 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -2101,7 +2101,7 @@ def retrieve_study_metadata( study_instance_uid: str Study Instance UID additional_params: Union[Dict[str, Any], None], optional - Additional HTTP GET query parameters + Additional HTTP GET query parameters Returns ------- @@ -3013,10 +3013,10 @@ def _get_instance_frames( url += f'/frames/{frame_list}' if media_types is None: return self._http_get_multipart( - url, + url, stream=stream, params=additional_params - ) + ) common_media_types = self._get_common_media_types(media_types) if len(common_media_types) > 1: diff --git a/tests/test_web.py b/tests/test_web.py index 35fe166..55cf95f 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1,5 +1,4 @@ import json -from operator import add import xml.etree.ElementTree as ET from io import BytesIO from http import HTTPStatus @@ -137,18 +136,24 @@ def test_search_for_studies(httpserver, client, cache_dir): ) -def test_search_for_studies_with_additional_params(httpserver, client, cache_dir): +def test_search_for_studies_with_additional_params( + httpserver, + client, + cache_dir +): cache_filename = str(cache_dir.joinpath('search_for_studies.json')) with open(cache_filename, 'r') as f: content = f.read() parsed_content = json.loads(content) headers = {'content-type': 'application/dicom+json'} httpserver.serve_content(content=content, code=200, headers=headers) - params = {"key1" : ["value1", "value2"], "key2" : "value3"} + params = {"key1": ["value1", "value2"], "key2": "value3"} assert client.search_for_studies(additional_params=params) == parsed_content request = httpserver.requests[0] assert request.path == '/studies' - assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) assert all( mime[0] in ('application/json', 'application/dicom+json') for mime in request.accept_mimetypes @@ -236,18 +241,24 @@ def test_search_for_series(httpserver, client, cache_dir): ) -def test_search_for_series_with_additional_params(httpserver, client, cache_dir): +def test_search_for_series_with_additional_params( + httpserver, + client, + cache_dir +): cache_filename = str(cache_dir.joinpath('search_for_series.json')) with open(cache_filename, 'r') as f: content = f.read() parsed_content = json.loads(content) headers = {'content-type': 'application/dicom+json'} httpserver.serve_content(content=content, code=200, headers=headers) - params = {"key1" : ["value1", "value2"], "key2" : "value3"} + params = {"key1": ["value1", "value2"], "key2": "value3"} assert client.search_for_series(additional_params=params) == parsed_content request = httpserver.requests[0] assert request.path == '/series' - assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) assert all( mime[0] in ('application/json', 'application/dicom+json') for mime in request.accept_mimetypes @@ -325,18 +336,26 @@ def test_search_for_instances(httpserver, client, cache_dir): ) -def test_search_for_instances_with_additional_params(httpserver, client, cache_dir): +def test_search_for_instances_with_additional_params( + httpserver, + client, + cache_dir +): cache_filename = str(cache_dir.joinpath('search_for_instances.json')) with open(cache_filename, 'r') as f: content = f.read() parsed_content = json.loads(content) headers = {'content-type': 'application/dicom+json'} httpserver.serve_content(content=content, code=200, headers=headers) - params = {"key1" : ["value1", "value2"], "key2" : "value3"} - assert client.search_for_instances(additional_params=params) == parsed_content + params = {"key1": ["value1", "value2"], "key2": "value3"} + assert client.search_for_instances( + additional_params=params + ) == parsed_content request = httpserver.requests[0] assert request.path == '/instances' - assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) assert all( mime[0] in ('application/json', 'application/dicom+json') for mime in request.accept_mimetypes @@ -435,23 +454,32 @@ def test_retrieve_instance_metadata(httpserver, client, cache_dir): ) -def test_retrieve_instance_metadata_with_additional_params(httpserver, client, cache_dir): +def test_retrieve_instance_metadata_with_additional_params( + httpserver, + client, + cache_dir +): cache_filename = str(cache_dir.joinpath('retrieve_instance_metadata.json')) with open(cache_filename, 'r') as f: content = f.read() parsed_content = json.loads(content) headers = {'content-type': 'application/dicom+json'} httpserver.serve_content(content=content, code=200, headers=headers) - params = {"key1" : ["value1", "value2"], "key2" : "value3"} + params = {"key1": ["value1", "value2"], "key2": "value3"} study_instance_uid = '1.2.3' series_instance_uid = '1.2.4' sop_instance_uid = '1.2.5' result = client.retrieve_instance_metadata( - study_instance_uid, series_instance_uid, sop_instance_uid, additional_params=params + study_instance_uid, + series_instance_uid, + sop_instance_uid, + additional_params=params ) assert result == parsed_content[0] request = httpserver.requests[0] - assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) expected_path = ( f'/studies/{study_instance_uid}' f'/series/{series_instance_uid}' @@ -603,7 +631,12 @@ def test_retrieve_instance(httpserver, client, cache_dir): assert request.path == expected_path assert request.accept_mimetypes[0][0][:43] == headers['content-type'][:43] -def test_retrieve_instance_with_additional_params(httpserver, client, cache_dir): + +def test_retrieve_instance_with_additional_params( + httpserver, + client, + cache_dir +): cache_filename = str(cache_dir.joinpath('file.dcm')) with open(cache_filename, 'rb') as f: data = f.read() @@ -621,19 +654,24 @@ def test_retrieve_instance_with_additional_params(httpserver, client, cache_dir) content_type=headers['content-type'] ) httpserver.serve_content(content=message, code=200, headers=headers) - params = {"key1" : ["value1", "value2"], "key2" : "value3"} + params = {"key1": ["value1", "value2"], "key2": "value3"} study_instance_uid = '1.2.3' series_instance_uid = '1.2.4' sop_instance_uid = '1.2.5' response = client.retrieve_instance( - study_instance_uid, series_instance_uid, sop_instance_uid, additional_params=params + study_instance_uid, + series_instance_uid, + sop_instance_uid, + additional_params=params ) with BytesIO() as fp: pydicom.dcmwrite(fp, response) raw_result = fp.getvalue() assert raw_result == data request = httpserver.requests[0] - assert request.query_string.decode() == 'key1=value1&key1=value2&key2=value3' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) expected_path = ( f'/studies/{study_instance_uid}' f'/series/{series_instance_uid}' From 5d4a25f7cb90def9d8cccf0226c6074953fdd6f7 Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Thu, 8 May 2025 14:47:29 +0200 Subject: [PATCH 5/8] fixing docstring --- src/dicomweb_client/web.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index 171f3fc..aac6ab4 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -1659,7 +1659,7 @@ def search_for_studies( Study representations (see `Study Result Attributes `_) - Notes + Note ---- - The server may only return a subset of search results. In this case, a warning will notify the client that there are remaining results. @@ -2207,7 +2207,7 @@ def search_for_series( Series representations (see `Series Result Attributes `_) - Notes + Note ---- - The server may only return a subset of search results. In this case, a warning will notify the client that there are remaining results. From 40ffe7f49a52c97cc9f0b84177da50d676f1c03c Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Fri, 9 May 2025 09:57:17 +0200 Subject: [PATCH 6/8] adding additional params to retrieve_study, iter_study, retrieve_series, iter_series --- src/dicomweb_client/web.py | 28 +++++++++--- tests/test_web.py | 91 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index aac6ab4..bdbc2a6 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -2013,6 +2013,7 @@ def retrieve_study( self, study_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> List[pydicom.dataset.Dataset]: """Retrieve all instances of a study. @@ -2023,6 +2024,8 @@ def retrieve_study( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2044,7 +2047,8 @@ def retrieve_study( self._get_study( study_instance_uid=study_instance_uid, media_types=media_types, - stream=False + stream=False, + additional_params=additional_params ) ) @@ -2052,6 +2056,7 @@ def iter_study( self, study_instance_uid: str, media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Iterate over all instances of a study. @@ -2062,6 +2067,8 @@ def iter_study( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2086,7 +2093,8 @@ def iter_study( return self._get_study( study_instance_uid=study_instance_uid, media_types=media_types, - stream=True + stream=True, + additional_params=additional_params ) def retrieve_study_metadata( @@ -2317,7 +2325,8 @@ def retrieve_series( self, study_instance_uid: str, series_instance_uid: str, - media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None + media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> List[pydicom.dataset.Dataset]: """Retrieve all instances of a series. @@ -2330,6 +2339,8 @@ def retrieve_series( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2352,7 +2363,8 @@ def retrieve_series( study_instance_uid=study_instance_uid, series_instance_uid=series_instance_uid, media_types=media_types, - stream=False + stream=False, + additional_params=additional_params ) ) @@ -2360,7 +2372,8 @@ def iter_series( self, study_instance_uid: str, series_instance_uid: str, - media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None + media_types: Optional[Tuple[Union[str, Tuple[str, str]], ...]] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> Iterator[pydicom.dataset.Dataset]: """Iterate over all instances of a series. @@ -2373,6 +2386,8 @@ def iter_series( media_types: Union[Tuple[Union[str, Tuple[str, str]], ...], None], optional Acceptable media types and optionally the UIDs of the acceptable transfer syntaxes + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP GET query parameters Returns ------- @@ -2398,7 +2413,8 @@ def iter_series( study_instance_uid=study_instance_uid, series_instance_uid=series_instance_uid, media_types=media_types, - stream=True + stream=True, + additional_params=additional_params ) def retrieve_series_metadata( diff --git a/tests/test_web.py b/tests/test_web.py index 55cf95f..9edc7e1 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -554,6 +554,53 @@ def test_iter_series(client, httpserver, cache_dir): assert len(response) == n_resources +def test_iter_series_with_additional_params(client, httpserver, cache_dir): + cache_filename = str(cache_dir.joinpath('file.dcm')) + with open(cache_filename, 'rb') as f: + data = f.read() + + n_resources = 3 + chunk_size = 10**3 + media_type = 'application/dicom' + boundary = 'boundary' + headers = { + 'content-type': ( + 'multipart/related; ' + f'type="{media_type}"; ' + f'boundary="{boundary}"' + ), + 'transfer-encoding': 'chunked' + } + params = {"key1": ["value1", "value2"], "key2": "value3"} + + message = DICOMwebClient._encode_multipart_message( + content=[data for _ in range(n_resources)], + content_type=headers['content-type'] + ) + chunked_message = _chunk_message(message, chunk_size) + + httpserver.serve_content(content=chunked_message, code=200, headers=headers) + study_uid = '1.2.3' + series_uid = '1.2.4' + iterator = client.iter_series( + study_uid, series_uid, additional_params=params + ) + assert isinstance(iterator, Generator) + response = list(iterator) + for instance in response: + with BytesIO() as fp: + pydicom.dcmwrite(fp, instance) + raw_result = fp.getvalue() + assert raw_result == data + request = httpserver.requests[0] + assert request.path == f'/studies/{study_uid}/series/{series_uid}' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + assert request.accept_mimetypes[0][0][:43] == headers['content-type'][:43] + assert len(response) == n_resources + + def test_retrieve_series(client, httpserver, cache_dir): cache_filename = str(cache_dir.joinpath('file.dcm')) with open(cache_filename, 'rb') as f: @@ -594,6 +641,50 @@ def test_retrieve_series(client, httpserver, cache_dir): assert len(response) == n_resources +def test_retrieve_series_with_additional_params(client, httpserver, cache_dir): + cache_filename = str(cache_dir.joinpath('file.dcm')) + with open(cache_filename, 'rb') as f: + data = f.read() + + n_resources = 3 + media_type = 'application/dicom' + boundary = 'boundary' + headers = { + 'content-type': ( + 'multipart/related; ' + f'type="{media_type}"; ' + f'boundary="{boundary}"' + ), + } + params = {"key1": ["value1", "value2"], "key2": "value3"} + message = DICOMwebClient._encode_multipart_message( + content=[data for _ in range(n_resources)], + content_type=headers['content-type'] + ) + httpserver.serve_content(content=message, code=200, headers=headers) + study_instance_uid = '1.2.3' + series_instance_uid = '1.2.4' + response = client.retrieve_series( + study_instance_uid, series_instance_uid, additional_params=params + ) + for resource in response: + with BytesIO() as fp: + pydicom.dcmwrite(fp, resource) + raw_result = fp.getvalue() + assert raw_result == data + request = httpserver.requests[0] + expected_path = ( + f'/studies/{study_instance_uid}' + f'/series/{series_instance_uid}' + ) + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + assert request.path == expected_path + assert request.accept_mimetypes[0][0][:43] == headers['content-type'][:43] + assert len(response) == n_resources + + def test_retrieve_instance(httpserver, client, cache_dir): cache_filename = str(cache_dir.joinpath('file.dcm')) with open(cache_filename, 'rb') as f: From 0d3667c3bea235235fe9a9cf91240d46fab82bb9 Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Fri, 9 May 2025 11:12:51 +0200 Subject: [PATCH 7/8] adding additional_params to delete methods (delete_study, delete_series, delete_instance) --- src/dicomweb_client/web.py | 38 +++++++++++++++-- tests/test_web.py | 84 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index bdbc2a6..a95f572 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -21,7 +21,7 @@ Union, Tuple, ) -from urllib.parse import urlparse +from urllib.parse import urlencode, urlparse from warnings import warn from xml.etree.ElementTree import ( Element, @@ -2126,13 +2126,19 @@ def retrieve_study_metadata( url += '/metadata' return self._http_get_application_json(url, params=additional_params) - def delete_study(self, study_instance_uid: str) -> None: + def delete_study( + self, + study_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None + ) -> None: """Delete all instances of a study. Parameters ---------- study_instance_uid: str Study Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP DELETE query parameters Note ---- @@ -2149,6 +2155,12 @@ def delete_study(self, study_instance_uid: str) -> None: 'Study Instance UID is required for deletion of a study.' ) url = self._get_studies_url(_Transaction.DELETE, study_instance_uid) + # Append query string if additional_params is provided + if additional_params: + additional_params_query_string = urlencode( + additional_params, doseq=True + ) + url += f'?{additional_params_query_string}' self._http_delete(url) def _assert_uid_format(self, uid: str) -> None: @@ -2541,7 +2553,8 @@ def retrieve_series_rendered( def delete_series( self, study_instance_uid: str, - series_instance_uid: str + series_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None ) -> None: """Delete all instances of a series. @@ -2551,6 +2564,8 @@ def delete_series( Study Instance UID series_instance_uid: str Series Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP DELETE query parameters Note ---- @@ -2579,6 +2594,12 @@ def delete_series( study_instance_uid, series_instance_uid ) + # Append query string if additional_params is provided + if additional_params: + additional_params_query_string = urlencode( + additional_params, doseq=True + ) + url += f'?{additional_params_query_string}' self._http_delete(url) def search_for_instances( @@ -2796,7 +2817,8 @@ def delete_instance( self, study_instance_uid: str, series_instance_uid: str, - sop_instance_uid: str + sop_instance_uid: str, + additional_params: Optional[Dict[str, Any]] = None ) -> None: """Delete specified instance. @@ -2808,6 +2830,8 @@ def delete_instance( Series Instance UID sop_instance_uid: str SOP Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP DELETE query parameters Note ---- @@ -2837,6 +2861,12 @@ def delete_instance( series_instance_uid, sop_instance_uid ) + # Append query string if additional_params is provided + if additional_params: + additional_params_query_string = urlencode( + additional_params, doseq=True + ) + url += f'?{additional_params_query_string}' self._http_delete(url) def retrieve_instance_metadata( diff --git a/tests/test_web.py b/tests/test_web.py index 9edc7e1..275eb58 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1342,6 +1342,31 @@ def test_delete_study_error(httpserver, client, cache_dir): assert request.method == 'DELETE' +def test_delete_study_error_with_additional_params( + httpserver, client, cache_dir +): + study_instance_uid = '1.2.3' + httpserver.serve_content( + content='', + code=HTTPStatus.METHOD_NOT_ALLOWED, + headers='' + ) + params = {"key1": ["value1", "value2"], "key2": "value3"} + with pytest.raises(HTTPError): + client.delete_study( + study_instance_uid=study_instance_uid, + additional_params=params + ) + assert len(httpserver.requests) == 1 + request = httpserver.requests[0] + expected_path = f'/studies/{study_instance_uid}' + assert request.path == expected_path + assert request.method == 'DELETE' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + + def test_delete_series_error(httpserver, client, cache_dir): study_instance_uid = '1.2.3' series_instance_uid = '1.2.4' @@ -1363,6 +1388,34 @@ def test_delete_series_error(httpserver, client, cache_dir): assert request.method == 'DELETE' +def test_delete_series_error_with_additional_params( + httpserver, client, cache_dir +): + study_instance_uid = '1.2.3' + series_instance_uid = '1.2.4' + httpserver.serve_content( + content='', + code=HTTPStatus.METHOD_NOT_ALLOWED, + headers='' + ) + params = {"key1": ["value1", "value2"], "key2": "value3"} + with pytest.raises(HTTPError): + client.delete_series(study_instance_uid=study_instance_uid, + series_instance_uid=series_instance_uid, + additional_params=params) + assert len(httpserver.requests) == 1 + request = httpserver.requests[0] + expected_path = ( + f'/studies/{study_instance_uid}' + f'/series/{series_instance_uid}' + ) + assert request.path == expected_path + assert request.method == 'DELETE' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + + def test_delete_instance_error(httpserver, client, cache_dir): study_instance_uid = '1.2.3' series_instance_uid = '1.2.4' @@ -1387,6 +1440,37 @@ def test_delete_instance_error(httpserver, client, cache_dir): assert request.method == 'DELETE' +def test_delete_instance_error_with_additional_params( + httpserver, client, cache_dir +): + study_instance_uid = '1.2.3' + series_instance_uid = '1.2.4' + sop_instance_uid = '1.2.5' + httpserver.serve_content( + content='', + code=HTTPStatus.METHOD_NOT_ALLOWED, + headers='' + ) + params = {"key1": ["value1", "value2"], "key2": "value3"} + with pytest.raises(HTTPError): + client.delete_instance(study_instance_uid=study_instance_uid, + series_instance_uid=series_instance_uid, + sop_instance_uid=sop_instance_uid, + additional_params=params) + assert len(httpserver.requests) == 1 + request = httpserver.requests[0] + expected_path = ( + f'/studies/{study_instance_uid}' + f'/series/{series_instance_uid}' + f'/instances/{sop_instance_uid}' + ) + assert request.path == expected_path + assert request.method == 'DELETE' + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + + def test_load_json_dataset_da(httpserver, client, cache_dir): value = ['2018-11-21'] dicom_json = { From 9af0d5e95bea39cd20568aa52cfb25e261f40ab8 Mon Sep 17 00:00:00 2001 From: Sumantra Sharma Date: Fri, 9 May 2025 13:19:45 +0200 Subject: [PATCH 8/8] adding additional params to POST method (store_instances) --- src/dicomweb_client/web.py | 11 +++++++- tests/test_web.py | 55 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/dicomweb_client/web.py b/src/dicomweb_client/web.py index a95f572..3da75f4 100644 --- a/src/dicomweb_client/web.py +++ b/src/dicomweb_client/web.py @@ -2778,7 +2778,8 @@ def retrieve_instance( def store_instances( self, datasets: Sequence[pydicom.dataset.Dataset], - study_instance_uid: Optional[str] = None + study_instance_uid: Optional[str] = None, + additional_params: Optional[Dict[str, Any]] = None ) -> pydicom.dataset.Dataset: """Store instances. @@ -2788,6 +2789,8 @@ def store_instances( Instances that should be stored study_instance_uid: Union[str, None], optional Study Instance UID + additional_params: Union[Dict[str, Any], None], optional + Additional HTTP POST query parameters Returns ------- @@ -2807,6 +2810,12 @@ def _iter_encoded_datasets(datasets): message += f' of study "{study_instance_uid}"' logger.info(message) url = self._get_studies_url(_Transaction.STORE, study_instance_uid) + # Append query string if additional_params is provided + if additional_params: + additional_params_query_string = urlencode( + additional_params, doseq=True + ) + url += f'?{additional_params_query_string}' encoded_datasets = _iter_encoded_datasets(datasets) return self._http_post_multipart_application_dicom( url, diff --git a/tests/test_web.py b/tests/test_web.py index 275eb58..2de4893 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1307,6 +1307,36 @@ def test_store_instance_error_with_retries(httpserver, client, cache_dir): ) +def test_store_instance_error_with_retries_and_additional_params( + httpserver, client, cache_dir +): + dataset = pydicom.Dataset.from_json({}) + dataset.is_little_endian = True + dataset.is_implicit_VR = True + max_attempts = 2 + client.set_http_retry_params( + retry=True, + max_attempts=max_attempts, + wait_exponential_multiplier=10 + ) + httpserver.serve_content( + content='', + code=HTTPStatus.REQUEST_TIMEOUT, + headers='' + ) + params = {"key1": ["value1", "value2"], "key2": "value3"} + with pytest.raises(RetryError): + client.store_instances([dataset], additional_params=params) + assert len(httpserver.requests) == max_attempts + request = httpserver.requests[0] + assert request.headers['Content-Type'].startswith( + 'multipart/related; type="application/dicom"' + ) + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + + def test_store_instance_error_with_no_retries(httpserver, client, cache_dir): dataset = pydicom.Dataset.from_json({}) dataset.is_little_endian = True @@ -1326,6 +1356,31 @@ def test_store_instance_error_with_no_retries(httpserver, client, cache_dir): ) +def test_store_instance_error_with_no_retries_and_additional_params( + httpserver, client, cache_dir +): + dataset = pydicom.Dataset.from_json({}) + dataset.is_little_endian = True + dataset.is_implicit_VR = True + client.set_http_retry_params(retry=False) + httpserver.serve_content( + content='', + code=HTTPStatus.REQUEST_TIMEOUT, + headers='' + ) + params = {"key1": ["value1", "value2"], "key2": "value3"} + with pytest.raises(HTTPError): + client.store_instances([dataset], additional_params=params) + assert len(httpserver.requests) == 1 + request = httpserver.requests[0] + assert request.headers['Content-Type'].startswith( + 'multipart/related; type="application/dicom"' + ) + assert request.query_string.decode() == ( + 'key1=value1&key1=value2&key2=value3' + ) + + def test_delete_study_error(httpserver, client, cache_dir): study_instance_uid = '1.2.3' httpserver.serve_content(