diff --git a/CHANGES.md b/CHANGES.md index 09cc39d..744771b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,12 +4,15 @@ The released versions correspond to PyPi releases. ## [Version 0.4.0] (Unreleased) ### Fixes -* condition parser: multiple or expressions handled correctly +* Condition parser: multiple or expressions handled correctly * Condition parser: handle a few more complicated conditions * Condition parser: handle conditions without values, see [#15](../.. /issues/15) * Condition parser: fixed handling of "zero" and "non-zero" values see [#17](../.. /issues/17) +* Condition parser: handle a few more simple expressions +* Condition parser: ignore equality conditions with missing values + (caused crash, see [#20](../.. /issues/20) ### Changes * Removed support for Python 3.6 diff --git a/dicom_validator/spec_reader/condition.py b/dicom_validator/spec_reader/condition.py index a4a60d6..98c4d24 100644 --- a/dicom_validator/spec_reader/condition.py +++ b/dicom_validator/spec_reader/condition.py @@ -15,6 +15,8 @@ class Condition: otherwise not 'MU': the object is mandatory if the condition is fulfilled, otherwise is user defined + 'MC': the object is mandatory if the condition is fulfilled, + otherwise another condition will be checked tag: the ID of the required tag in the form '(####,####)' or None index: the index of the tag for multi-valued tags or 0 values: a list of values the tag shall have if the condition @@ -38,6 +40,9 @@ def __init__(self, ctype: Optional[str] = None, self.or_conditions: List[Condition] = [] self.other_condition: Optional[Condition] = None + def __repr__(self): + return f"Condition type={self.type} op='{self.operator}' tag={self.tag} values={self.values}" + @classmethod def read_condition(cls, condition_dict: Dict, condition: Optional["Condition"] = None) -> "Condition": diff --git a/dicom_validator/spec_reader/condition_parser.py b/dicom_validator/spec_reader/condition_parser.py index 92cb03a..c24c2aa 100644 --- a/dicom_validator/spec_reader/condition_parser.py +++ b/dicom_validator/spec_reader/condition_parser.py @@ -31,10 +31,13 @@ class ConditionParser: r'(?P\([\dA-Fa-f]{4},[\dA-Fa-f]{4}\))?(,? Value (?P\d))?$') operators = OrderedDict([ + (' is present and the value is ', '='), + (' is present and has a value of ', '='), (' is greater than ', '>'), (' is present and equals ', '='), (' is present with a value of ', '='), - (' value is ', '='), + (' is set to ', '='), + (' equals one of the following values: ', '='), (' has a value of more than ', '>'), (' has a value greater than ', '>'), (' has a value of ', '='), @@ -43,16 +46,17 @@ class ConditionParser: (' equals other than ', '!='), (' equals ', '='), (' is other than ', '!='), - (' is present and the value is ', '='), - (' is present and has a value of ', '='), (' is one of the following: ', '='), (' is present and has a value', '++'), (' is present', '+'), + (' value is not ', '!='), + (' value is ', '='), (' is sent', '+'), (' is not sent', '-'), (' is not present', '-'), (' is absent', '-'), (' is not equal to ', '!='), + (' is equal to ', '='), (' is not ', '!='), (' is ', '='), (' is: ', '='), @@ -115,9 +119,18 @@ def _parse_tag_expression( if operator in ('=', '!=', '>', '<'): values, rest = self._parse_tag_values(rest) # fixup special values - if values and values[0].startswith('non-zero'): - operator = '!=' - values = ['0'] if values[0] == 'non-zero' else [''] + if values: + if values[0].startswith('non-zero'): + operator = '!=' + values = ['0'] if values[0] == 'non-zero' else [''] + elif values[0].startswith('non-null'): + operator = '++' + values = [] + elif values[0].startswith('zero-length'): + values = [''] + else: + # failed to parse mandatory values - ignore the condition + return Condition(ctype='U'), None elif operator == '=>': value_string, rest = self._split_value_part(rest) tag, _ = self._parse_tag(value_string) @@ -203,7 +216,7 @@ def _parse_tag_values( def _split_value_part(self, value_string: str) -> Tuple[str, str]: value_string = value_string.strip() end_index = self._end_index_for_stop_chars( - value_string, [';', '.', ', and ', ' and ']) + value_string, [';', '.', ', and ', ' and ', ':']) return value_string[:end_index], value_string[end_index:] @staticmethod @@ -232,13 +245,17 @@ def _get_const_value(self, value: str) -> Tuple[Optional[str], str]: value = value.strip() if value[0] == value[-1] == '"': return value[1:-1], '' - if re.match('^[A-Z0-9_ ]+$', value) is not None: + if re.match('^[A-Z0-9][A-Za-z0-9_ ]*$', value) is not None: return value, '' + # sometimes a value explanation is present in scopes + match = re.match(r'^([A-Z0-9_ ]+)\([A-Za-z ]+\)+$', value) + if match is not None: + return match.group(1).strip(), '' if value == 'zero length': return '', '' if value == 'zero': return '0', '' - if value in ('non-zero', 'non-zero length'): + if value in ('non-zero', 'non-zero length', 'non-null', 'zero-length'): return value, '' match = re.match(r'^.* \(\"([\d.]+)\"\)(.*)$', value) if match is not None: diff --git a/dicom_validator/spec_reader/tests/test_condition_parser.py b/dicom_validator/spec_reader/tests/test_condition_parser.py index 4c94032..24bf14d 100644 --- a/dicom_validator/spec_reader/tests/test_condition_parser.py +++ b/dicom_validator/spec_reader/tests/test_condition_parser.py @@ -45,15 +45,13 @@ def test_ignore_condition_without_tag(self): ) self.assertEqual('U', result.type) - def test_handle_condition_without_value(self): + def test_ignore_condition_without_value(self): # regression test for #15 result = self.parser.parse( 'required if Selector Attribute (0072,0026) is nested in ' 'one or more Sequences or is absent.' ) - self.assertEqual('MN', result.type) - self.assertEqual('(0072,0026)', result.tag) - self.assertEqual('=', result.operator) + self.assertEqual('U', result.type) class SimpleConditionParserTest(ConditionParserTest): @@ -135,6 +133,14 @@ def test_required_only_if(self): self.assertEqual('-', result.operator) self.assertEqual([], result.values) + def test_values_failed_parsing(self): + """Regression test for #20 (if the value could not be parsed ignore the condition)""" + result = self.parser.parse( + 'Required if Constraint Violation Significance (0082,0036) ' + 'is only significant under certain conditions.' + ) + self.assertEqual('U', result.type) + class ValueConditionParserTest(ConditionParserTest): def test_equality_tag(self): @@ -318,6 +324,14 @@ def test_present_with_value(self): self.assertEqual('=', result.operator) self.assertEqual(['AS'], result.values) + def test_value_is_not(self): + result = self.parser.parse( + 'Required if Shadow Style (0070,0244) value is not OFF.' + ) + self.assertEqual('MN', result.type) + self.assertEqual('!=', result.operator) + self.assertEqual(['OFF'], result.values) + def test_other_than(self): result = self.parser.parse( 'Required if Decay Correction (0054,1102) is other than NONE.' @@ -335,6 +349,14 @@ def test_not_equal_to(self): self.assertEqual('!=', result.operator) self.assertEqual(['UNDEFINED'], result.values) + def test_equal_to(self): + result = self.parser.parse( + 'Required if Blending Mode (0070,1B06) is equal to FOREGROUND.' + ) + self.assertEqual('MN', result.type) + self.assertEqual('=', result.operator) + self.assertEqual(['FOREGROUND'], result.values) + def test_present_with_value_of(self): result = self.parser.parse( 'Required if Partial View ' @@ -361,6 +383,26 @@ def test_non_zero(self): self.assertEqual('!=', result.operator) self.assertEqual(['0'], result.values) + def test_non_null(self): + result = self.parser.parse( + 'Required if value Transfer Tube Number (300A,02A2) is non-null.' + ) + assert result.type == 'MN' + assert result.tag == '(300A,02A2)' + assert result.operator == '++' + self.assertEqual([], result.values) + + def test_zero_length(self): + result = self.parser.parse( + 'Required if Material ID (300A,00E1) is zero-length. ' + 'May be present if Material ID (300A,00E1) is non-zero length.' + ) + assert result.type == 'MC' + assert result.tag == '(300A,00E1)' + assert result.operator == '=' + self.assertEqual([''], result.values) + assert result.other_condition is not None + def test_greater_than_zero(self): result = self.parser.parse( 'Required if Number of Beams (300A,0080) is greater than zero' @@ -386,6 +428,51 @@ def test_is_not_zero_length(self): self.assertEqual('!=', result.operator) self.assertEqual([''], result.values) + def test_equal_sign(self): + result = self.parser.parse( + 'Required if Pixel Component Organization = Bit aligned.' + ) + self.assertEqual('MN', result.type) + self.assertEqual('(0018,6044)', result.tag) + self.assertEqual('=', result.operator) + self.assertEqual(['Bit aligned'], result.values) + + def test_value_has_explanation(self): + result = self.parser.parse( + 'Required if Conversion Type (0008,0064) is DF (Digitized Film).' + ) + self.assertEqual('MN', result.type) + self.assertEqual('(0008,0064)', result.tag) + self.assertEqual('=', result.operator) + self.assertEqual(['DF'], result.values) + + def test_values_have_explanation(self): + result = self.parser.parse( + 'Required if Conversion Type (0008,0064) is SD (Scanned Document) or SI (Scanned Image).' + ) + self.assertEqual('MN', result.type) + self.assertEqual('(0008,0064)', result.tag) + self.assertEqual('=', result.operator) + self.assertEqual(['SD', 'SI'], result.values) + + def test_is_with_colon_after_value(self): + result = self.parser.parse( + 'Required if the value of Reformatting Operation Type (0072,0510) is 3D_RENDERING:' + ) + self.assertEqual('MN', result.type) + self.assertEqual('(0072,0510)', result.tag) + self.assertEqual('=', result.operator) + self.assertEqual(['3D_RENDERING'], result.values) + + def test_is_set_to(self): + result = self.parser.parse( + 'Required if Ophthalmic Volumetric Properties Flag (0022,1622) is set to YES. May be present otherwise.' + ) + self.assertEqual('MU', result.type) + self.assertEqual('(0022,1622)', result.tag) + self.assertEqual('=', result.operator) + self.assertEqual(['YES'], result.values) + class NotMandatoryConditionParserTest(ConditionParserTest): def test_default(self): diff --git a/dicom_validator/tests/fixtures/2021d/json/dict_info.json b/dicom_validator/tests/fixtures/2021d/json/dict_info.json index 634161b..9c04d91 100644 --- a/dicom_validator/tests/fixtures/2021d/json/dict_info.json +++ b/dicom_validator/tests/fixtures/2021d/json/dict_info.json @@ -47,6 +47,12 @@ "vm": "1", "vr": "PN" }, + "(0018,6044)": { + "name": "Pixel Component Organization", + "prop": "", + "vm": "1", + "vr": "US" + }, "(0028,0008)": { "name": "Number of Frames", "prop": "",