[MRG] Add warning when setting element values with unknown keyword

doc/reference/config.rst

@@ -25,4 +25,5 @@ Configuration Options (:mod:`pydicom.config`)
    use_DS_decimal
    use_IS_numpy
    use_DS_numpy
-
+   APPLY_J2K_CORRECTIONS
+   INVALID_KEYWORD_BEHAVIOR

doc/release_notes/v2.1.0.rst

@@ -41,6 +41,11 @@ Enhancements
   the image data is converted to signed (:issue:`1149`)
 * Added :attr:`~pydicom.uid.UID.keyword` property for the new UID keywords
   in version 2020d of the DICOM Standard
+* Added testing of the variable names used when setting
+  :class:`~pydicom.dataset.Dataset` attributes and
+  :attr:`~pydicom.config.INVALID_KEYWORD_BEHAVIOR` config option to allow
+  customizing the behaviour when a camel case variable name is used that isn't
+  a known element keyword (:issue:`1014`)
 
 Changes
 .......

pydicom/config.py

@@ -292,6 +292,30 @@ def needs_to_convert_to_RGB(ds):
 values within the dataset when applying corrections.
 """
 
+INVALID_KEYWORD_BEHAVIOR = "WARN"
+"""Control the behaviour when setting a :class:`~pydicom.dataset.Dataset`
+attribute that's not a known element keyword.
+
+.. versionadded:: 2.1
+
+If ``"WARN"`` (default), then warn when an element value is set using
+``Dataset.__setattr__()`` and the keyword is camel case but doesn't match a
+known DICOM element keyword. If ``"RAISE"`` then raise a :class:`ValueError`
+exception. If ``"IGNORE"`` then neither warn nor raise.
+
+Examples
+--------
+
+>>> from pydicom import config
+>>> config.INVALID_KEYWORD_BEHAVIOR = "WARN"
+>>> ds = Dataset()
+>>> ds.PatientName = "Citizen^Jan"  # OK
+>>> ds.PatientsName = "Citizen^Jan"
+../pydicom/dataset.py:1895: UserWarning: Camel case attribute 'PatientsName'
+used which is not in the element keyword data dictionary
+
+"""
+
 
 def debug(debug_on=True, default_handler=True):
     """Turn on/off debugging of DICOM file reading and writing.

pydicom/dataset.py

@@ -22,6 +22,7 @@
 import json
 import os
 import os.path
+import re
 from typing import Generator, TYPE_CHECKING
 import warnings
 
@@ -1889,6 +1890,17 @@ def __setattr__(self, name, value):
         elif name == "file_meta":
             self._set_file_meta(value)
         else:
+            # Warn if `name` is camel case but not a keyword
+            if _RE_CAMEL_CASE.match(name):
+                msg = (
+                    f"Camel case attribute '{name}' used which is not in the "
+                    "element keyword data dictionary"
+                )
+                if config.INVALID_KEYWORD_BEHAVIOR == "WARN":
+                    warnings.warn(msg)
+                elif config.INVALID_KEYWORD_BEHAVIOR == "ERROR":
+                    raise ValueError(msg)
+
             # name not in dicom dictionary - setting a non-dicom instance
             # attribute
             # XXX note if user mis-spells a dicom data_element - no error!!!
@@ -2499,3 +2511,11 @@ def __setitem__(self, key, value):
             )
 
         super().__setitem__(key, value)
+
+
+_RE_CAMEL_CASE = re.compile(
+    # Ensure mix of upper and lowercase and digits, no underscores
+    # If first character is lowercase ensure at least one uppercase char
+    "(?P<start>(^[A-Za-z])((?=.+?[A-Z])[A-Za-z0-9]+)|(^[A-Z])([A-Za-z0-9]+))"
+    "(?P<last>[A-za-z0-9][^_]$)"  # Last character is alphanumeric
+)

pydicom/tests/test_dataset.py

@@ -1,10 +1,13 @@
-# Copyright 2008-2018 pydicom authors. See LICENSE file for details.
+# Copyright 2008-2020 pydicom authors. See LICENSE file for details.
 """Unit tests for the pydicom.dataset module."""
+
 import copy
+import warnings
 
 import pytest
 
 import pydicom
+from pydicom import config
 from pydicom import dcmread
 from pydicom.data import get_testdata_file
 from pydicom.dataelem import DataElement, RawDataElement
@@ -164,7 +167,12 @@ def test_set_existing_data_element_by_name(self):
     def test_set_non_dicom(self):
         """Dataset: can set class instance property (non-dicom)."""
         ds = Dataset()
-        ds.SomeVariableName = 42
+        msg = (
+            r"Camel case attribute 'SomeVariableName' used which is not in "
+            r"the element keyword data dictionary"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            ds.SomeVariableName = 42
         assert hasattr(ds, 'SomeVariableName')
         assert 42 == ds.SomeVariableName
 
@@ -377,7 +385,12 @@ def test_dir(self):
         ds = self.ds
         ds.PatientName = "name"
         ds.PatientID = "id"
-        ds.NonDicomVariable = "junk"
+        msg = (
+            r"Camel case attribute 'NonDicomVariable' used which is not in "
+            r"the element keyword data dictionary"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            ds.NonDicomVariable = "junk"
         ds.add_new((0x18, 0x1151), "IS", 150)  # X-ray Tube Current
         ds.add_new((0x1111, 0x123), "DS", "42.0")  # private - no name in dir()
         expected = ['PatientID',
@@ -391,7 +404,12 @@ def test_dir_filter(self):
         ds = self.ds
         ds.PatientName = "name"
         ds.PatientID = "id"
-        ds.NonDicomVariable = "junk"
+        msg = (
+            r"Camel case attribute 'NonDicomVariable' used which is not in "
+            r"the element keyword data dictionary"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            ds.NonDicomVariable = "junk"
         ds.add_new((0x18, 0x1151), "IS", 150)  # X-ray Tube Current
         ds.add_new((0x1111, 0x123), "DS", "42.0")  # private - no name in dir()
         assert 'PatientID' in ds
@@ -546,11 +564,17 @@ def test_equality_unknown(self):
         """Dataset: equality returns correct value with extra members """
         # Non-element class members are ignored in equality testing
         d = Dataset()
-        d.SOPEustaceUID = '1.2.3.4'
+        msg = (
+            r"Camel case attribute 'SOPEustaceUID' used which is not in "
+            r"the element keyword data dictionary"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            d.SOPEustaceUID = '1.2.3.4'
         assert d == d
 
         e = Dataset()
-        e.SOPEustaceUID = '1.2.3.5'
+        with pytest.warns(UserWarning, match=msg):
+            e.SOPEustaceUID = '1.2.3.5'
         assert d == e
 
     def test_equality_inheritance(self):
@@ -1896,3 +1920,102 @@ def test_copy(self, copy_method):
         assert ds_copy.is_implicit_VR
         assert ds_copy.is_little_endian
         assert ds_copy.read_encoding == "utf-8"
+
+
+CAMEL_CASE = (
+    [  # Shouldn't warn
+        "Rows", "_Rows", "Rows_", "rows", "_rows", "__rows", "rows_", "ro_ws",
+        "rowds", "BitsStored", "bits_Stored", "Bits_Stored", "bits_stored",
+        "_BitsStored", "BitsStored_", "B_itsStored", "BitsS_tored",
+        "12LeadECG", "file_meta", "filename", "is_implicit_VR",
+        "is_little_endian", "preamble", "timestamp", "fileobj_type",
+        "patient_records", "_parent_encoding", "_dict", "is_decompressed",
+        "read_little_endian", "read_implicit_vr", "read_encoding", "parent",
+        "_private_blocks", "default_element_format", "indent_chars",
+        "default_sequence_element_format", "PatientName"
+    ],
+    [  # Should warn
+        "bitsStored", "BitSStored", "TwelveLeadECG", "SOPInstanceUId",
+        "PatientsName", "Rowds"
+    ]
+)
+
+
+@pytest.fixture
+def setattr_raise():
+    """Raise on Dataset.__setattr__() close keyword matches."""
+    config.INVALID_KEYWORD_BEHAVIOR = "ERROR"
+    yield
+    config.INVALID_KEYWORD_BEHAVIOR = "WARN"
+
+
+@pytest.fixture
+def setattr_ignore():
+    """Ignore Dataset.__setattr__() close keyword matches."""
+    config.INVALID_KEYWORD_BEHAVIOR = "IGNORE"
+    yield
+    config.INVALID_KEYWORD_BEHAVIOR = "WARN"
+
+
+def test_setattr_warns():
+    """"Test warnings for Dataset.__setattr__() for close matches."""
+    with pytest.warns(None) as record:
+        ds = Dataset()
+        assert len(record) == 0
+
+    for s in CAMEL_CASE[0]:
+        with pytest.warns(None) as record:
+            val = getattr(ds, s, None)
+            setattr(ds, s, val)
+            assert len(record) == 0
+
+    for s in CAMEL_CASE[1]:
+        msg = (
+            r"Camel case attribute '" + s + r"' used which is not in the "
+            r"element keyword data dictionary"
+        )
+        with pytest.warns(UserWarning, match=msg):
+            val = getattr(ds, s, None)
+            setattr(ds, s, None)
+
+
+def test_setattr_raises(setattr_raise):
+    """"Test exceptions for Dataset.__setattr__() for close matches."""
+    with pytest.warns(None) as record:
+        ds = Dataset()
+        assert len(record) == 0
+
+    for s in CAMEL_CASE[0]:
+        with pytest.warns(None) as record:
+            val = getattr(ds, s, None)
+            setattr(ds, s, val)
+            assert len(record) == 0
+
+    for s in CAMEL_CASE[1]:
+        msg = (
+            r"Camel case attribute '" + s + r"' used which is not in the "
+            r"element keyword data dictionary"
+        )
+        with pytest.raises(ValueError, match=msg):
+            val = getattr(ds, s, None)
+            setattr(ds, s, None)
+
+
+def test_setattr_no_warning(setattr_ignore):
+    """Test config.INVALID_KEYWORD_BEHAVIOR = 'IGNORE'"""
+    with pytest.warns(None) as record:
+        ds = Dataset()
+        assert len(record) == 0
+
+    for s in CAMEL_CASE[0]:
+        with pytest.warns(None) as record:
+            val = getattr(ds, s, None)
+            setattr(ds, s, val)
+            assert len(record) == 0
+
+    ds = Dataset()
+    for s in CAMEL_CASE[1]:
+        with pytest.warns(None) as record:
+            val = getattr(ds, s, None)
+            setattr(ds, s, None)
+            assert len(record) == 0