From 2157139007d5dc5067b785afbfbe27d10474745a Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Sat, 22 Apr 2023 12:45:26 +0100 Subject: [PATCH] RF: refactor find_private_element Neater and more readable version of find_private_element, extend tests. --- nibabel/nicom/tests/test_utils.py | 54 ++++++++++++++++++++----------- nibabel/nicom/utils.py | 26 ++++++--------- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/nibabel/nicom/tests/test_utils.py b/nibabel/nicom/tests/test_utils.py index 37dbcd7d19..ea3b999fad 100644 --- a/nibabel/nicom/tests/test_utils.py +++ b/nibabel/nicom/tests/test_utils.py @@ -4,7 +4,7 @@ from nibabel.optpkg import optional_package -from ..utils import find_private_section +from ..utils import find_private_section as fps from .test_dicomwrappers import DATA, DATA_PHILIPS pydicom, _, setup_module = optional_package('pydicom') @@ -13,37 +13,53 @@ def test_find_private_section_real(): # Find section containing named private creator information # On real data first - assert find_private_section(DATA, 0x29, 'SIEMENS CSA HEADER') == 0x1000 - assert find_private_section(DATA, 0x29, 'SIEMENS MEDCOM HEADER2') == 0x1100 - assert find_private_section(DATA_PHILIPS, 0x29, 'SIEMENS CSA HEADER') == None - # Make fake datasets + assert fps(DATA, 0x29, 'SIEMENS CSA HEADER') == 0x1000 + assert fps(DATA, 0x29, b'SIEMENS CSA HEADER') == 0x1000 + assert fps(DATA, 0x29, re.compile('SIEMENS CSA HEADER')) == 0x1000 + assert fps(DATA, 0x29, 'NOT A HEADER') is None + assert fps(DATA, 0x29, 'SIEMENS MEDCOM HEADER2') == 0x1100 + assert fps(DATA_PHILIPS, 0x29, 'SIEMENS CSA HEADER') == None + + +def test_find_private_section_fake(): + # Make and test fake datasets ds = pydicom.dataset.Dataset({}) + assert fps(ds, 0x11, 'some section') is None ds.add_new((0x11, 0x10), 'LO', b'some section') - assert find_private_section(ds, 0x11, 'some section') == 0x1000 - ds.add_new((0x11, 0x11), 'LO', b'anther section') + assert fps(ds, 0x11, 'some section') == 0x1000 + ds.add_new((0x11, 0x11), 'LO', b'another section') ds.add_new((0x11, 0x12), 'LO', b'third section') - assert find_private_section(ds, 0x11, 'third section') == 0x1200 - # Wrong 'OB' is acceptable for VM (should be 'LO') + assert fps(ds, 0x11, 'third section') == 0x1200 + # Technically incorrect 'OB' is acceptable for VM (should be 'LO') ds.add_new((0x11, 0x12), 'OB', b'third section') - assert find_private_section(ds, 0x11, 'third section') == 0x1200 + assert fps(ds, 0x11, 'third section') == 0x1200 # Anything else not acceptable ds.add_new((0x11, 0x12), 'PN', b'third section') - assert find_private_section(ds, 0x11, 'third section') is None + assert fps(ds, 0x11, 'third section') is None # The input (DICOM value) can be a string insteal of bytes ds.add_new((0x11, 0x12), 'LO', 'third section') - assert find_private_section(ds, 0x11, 'third section') == 0x1200 + assert fps(ds, 0x11, 'third section') == 0x1200 # Search can be bytes as well as string ds.add_new((0x11, 0x12), 'LO', b'third section') - assert find_private_section(ds, 0x11, b'third section') == 0x1200 + assert fps(ds, 0x11, b'third section') == 0x1200 # Search with string or bytes must be exact - assert find_private_section(ds, 0x11, b'third sectio') is None - assert find_private_section(ds, 0x11, 'hird sectio') is None + assert fps(ds, 0x11, b'third sectio') is None + assert fps(ds, 0x11, 'hird sectio') is None # The search can be a regexp - assert find_private_section(ds, 0x11, re.compile(r'third\Wsectio[nN]')) == 0x1200 + assert fps(ds, 0x11, re.compile(r'third\Wsectio[nN]')) == 0x1200 # No match -> None - assert find_private_section(ds, 0x11, re.compile(r'not third\Wsectio[nN]')) is None + assert fps(ds, 0x11, re.compile(r'not third\Wsectio[nN]')) is None # If there are gaps in the sequence before the one we want, that is OK ds.add_new((0x11, 0x13), 'LO', b'near section') - assert find_private_section(ds, 0x11, 'near section') == 0x1300 + assert fps(ds, 0x11, 'near section') == 0x1300 ds.add_new((0x11, 0x15), 'LO', b'far section') - assert find_private_section(ds, 0x11, 'far section') == 0x1500 + assert fps(ds, 0x11, 'far section') == 0x1500 + # More than one match - find the first. + assert fps(ds, 0x11, re.compile('(another|third) section')) == 0x1100 + # The signalling element number must be <= 0xFF + ds = pydicom.dataset.Dataset({}) + ds.add_new((0x11, 0xFF), 'LO', b'some section') + assert fps(ds, 0x11, 'some section') == 0xFF00 + ds = pydicom.dataset.Dataset({}) + ds.add_new((0x11, 0x100), 'LO', b'some section') + assert fps(ds, 0x11, 'some section') is None diff --git a/nibabel/nicom/utils.py b/nibabel/nicom/utils.py index 48a010903a..1610c49e9d 100644 --- a/nibabel/nicom/utils.py +++ b/nibabel/nicom/utils.py @@ -27,26 +27,20 @@ def find_private_section(dcm_data, group_no, creator): Returns ------- element_start : int - Element number at which named section starts + Element number at which named section starts. """ - is_regex = hasattr(creator, 'search') - if not is_regex: # assume string / bytes + if hasattr(creator, 'search'): + match_func = lambda x : creator.search(x) + else: # assume string / bytes creator = asstr(creator) - for element in dcm_data: # Assumed ordered by tag (groupno, elno) - grpno, elno = element.tag.group, element.tag.elem - if grpno > group_no: - break - if grpno != group_no: - continue + match_func = lambda x : x == creator + # Group elements assumed ordered by tag (groupno, elno) + for element in dcm_data.group_dataset(group_no): + elno = element.tag.elem if elno > 0xFF: break if element.VR not in ('LO', 'OB'): continue - name = asstr(element.value) - if is_regex: - if creator.search(name) is not None: - return elno * 0x100 - else: # string - needs exact match - if creator == name: - return elno * 0x100 + if match_func(asstr(element.value)): + return elno * 0x100 return None