Repository: pydicom/pydicom

Issue:
Print a warning when assigning a non-existing DICOM Tag
**Is your feature request related to a problem? Please describe.**
This problem is related to #1013
```python
import pydicom
ds = pydicom.Dataset()
ds.SegmentsOverlap = 'NO'  # (0x0062, 0x0013)
print(ds.to_json())
# {}
```
For non-existing tags in the DICOM dictionary, those assignments and reads don't work properly and you can only spot the difference when dumping the DICOM dataset.

**Describe the solution you'd like**
When the DICOM Tag lookup is not successful and the name is not another existing property/variable, then emit a warning or even an Exception should be thrown.


Discussion:
pep8speaks: Hello @scaramallion! Thanks for opening this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





* In the file [`pydicom/tests/test_dataset.py`](https://github.com/pydicom/pydicom/blob/e17a91382a4b668a70610ac09ef59bb512a8a68a/pydicom/tests/test_dataset.py):

> [Line 1943:1](https://github.com/pydicom/pydicom/blob/e17a91382a4b668a70610ac09ef59bb512a8a68a/pydicom/tests/test_dataset.py#L1943): [E302](https://duckduckgo.com/?q=pep8%20E302) expected 2 blank lines, found 1


codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=h1) Report
> Merging [#1199](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=desc) into [master](https://codecov.io/gh/pydicom/pydicom/commit/147641145c09cf38d6c796ed47427aaffb475872?el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pydicom/pydicom/pull/1199/graphs/tree.svg?width=650&height=150&src=pr&token=YciLV15Lwr)](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master    #1199   +/-   ##
=======================================
  Coverage   94.05%   94.06%           
=======================================
  Files          54       54           
  Lines        7975     7980    +5     
=======================================
+ Hits         7501     7506    +5     
  Misses        474      474           
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pydicom/config.py](https://codecov.io/gh/pydicom/pydicom/pull/1199/diff?src=pr&el=tree#diff-cHlkaWNvbS9jb25maWcucHk=) | `100.00% <100.00%> (ø)` | |
| [pydicom/dataset.py](https://codecov.io/gh/pydicom/pydicom/pull/1199/diff?src=pr&el=tree#diff-cHlkaWNvbS9kYXRhc2V0LnB5) | `98.18% <100.00%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=footer). Last update [1476411...08c5b4c](https://codecov.io/gh/pydicom/pydicom/pull/1199?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

scaramallion: OK, changes done

Patch:
diff --git a/doc/old/base_element.rst b/doc/old/base_element.rst
index 608d0c011..d687d1804 100644
--- a/doc/old/base_element.rst
+++ b/doc/old/base_element.rst
@@ -241,7 +241,7 @@ Tag
 elements using their keywords as illustrated in sections above.
 
 The :class:`~tag.BaseTag` class is derived from :class:`int`,
-so in effect, it is just a number with some extra behaviour:
+so in effect, it is just a number with some extra behavior:
 
   * :func:`~tag.Tag` is used to create instances of :class:`~tag.BaseTag` and
     enforces that the DICOM tag fits in the expected 4-byte (group,element)
diff --git a/doc/reference/config.rst b/doc/reference/config.rst
index 31f8f604c..a8d4ee162 100644
--- a/doc/reference/config.rst
+++ b/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
diff --git a/doc/release_notes/v2.1.0.rst b/doc/release_notes/v2.1.0.rst
index e03dcfa86..2bf5876f5 100644
--- a/doc/release_notes/v2.1.0.rst
+++ b/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 behavior when a camel case variable name is used that isn't
+  a known element keyword (:issue:`1014`)
 
 Changes
 .......
@@ -54,6 +59,9 @@ Changes
   become ``MultiFrame``, those with ``and`` in them become ``And``, and
   ``DICOSQuadrupoleResonanceQRStorage`` becomes
   ``DICOSQuadrupoleResonanceStorage``.
+* Invalid values used with the :class:`~pydicom.dataset.Dataset`
+  :func:`in<operator.__contains__>` now raise :class:`ValueError` rather than
+  returning ``False`` (:issue:`1200`)
 
 Fixes
 .....
diff --git a/pydicom/config.py b/pydicom/config.py
index e8b2bb39a..81fb4cc5d 100644
--- a/pydicom/config.py
+++ b/pydicom/config.py
@@ -292,6 +292,30 @@ using the pixel data handlers. If ``False`` then only rely on the element
 values within the dataset when applying corrections.
 """
 
+INVALID_KEYWORD_BEHAVIOR = "WARN"
+"""Control the behavior 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.
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index 8890dedd2..38c5f8829 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -22,6 +22,7 @@ from itertools import takewhile
 import json
 import os
 import os.path
+import re
 from typing import Generator, TYPE_CHECKING
 import warnings
 
@@ -475,13 +476,17 @@ class Dataset(dict):
             ``False`` otherwise.
         """
         try:
-            tag = Tag(name)
-        except (ValueError, OverflowError):
-            return False
-        # Test against None as (0000,0000) is a possible tag
-        if tag is not None:
-            return tag in self._dict
-        return name in self._dict  # will no doubt raise an exception
+            return Tag(name) in self._dict
+        except OverflowError as exc:
+            raise OverflowError(
+                "Invalid element tag value: tags have a maximum value of "
+                "(0xFFFF, 0xFFFF)"
+            ) from exc
+        except Exception as exc:
+            raise ValueError(
+                "Invalid value used with the 'in' operator: must be an "
+                "element tag as a 2-tuple or int, or an element keyword"
+            ) from exc
 
     def decode(self):
         """Apply character set decoding to the elements in the
@@ -1889,6 +1894,17 @@ class Dataset(dict):
         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 +2515,11 @@ class FileMetaDataset(Dataset):
             )
 
         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
+)
diff --git a/pydicom/util/codify.py b/pydicom/util/codify.py
index 3557cf2dd..5cdc3bae9 100644
--- a/pydicom/util/codify.py
+++ b/pydicom/util/codify.py
@@ -171,9 +171,9 @@ def code_sequence(dataelem,
         #    use that; if one with 'Number', use that, else start at 1
         index_keyword = seq_keyword.replace("Sequence", "") + "Index"
         number_keyword = seq_keyword.replace("Sequence", "") + "Number"
-        if index_keyword in ds:
+        if hasattr(ds, index_keyword):
             index_str = str(getattr(ds, index_keyword))
-        elif number_keyword in ds:
+        elif hasattr(ds, number_keyword):
             index_str = str(getattr(ds, number_keyword))
         else:
             index_str = str(i + 1)


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index b723751a2..2c3fcd77c 100644
--- a/pydicom/tests/test_dataset.py
+++ b/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
@@ -161,17 +164,27 @@ class TestDataset:
         self.ds.TreatmentMachineName = "unit999"  # change existing value
         assert "unit999" == self.ds[0x300a, 0x00b2].value
 
-    def test_set_non_dicom(self):
+    def test_set_non_dicom(self, setattr_warn):
         """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
 
     def test_membership(self):
         """Dataset: can test if item present by 'if <name> in dataset'."""
         assert 'TreatmentMachineName' in self.ds
-        assert 'Dummyname' not in self.ds
+        msg = (
+            r"Invalid value used with the 'in' operator: must be "
+            r"an element tag as a 2-tuple or int, or an element keyword"
+        )
+        with pytest.raises(ValueError, match=msg):
+            'Dummyname' not in self.ds
 
     def test_contains(self):
         """Dataset: can test if item present by 'if <tag> in dataset'."""
@@ -182,9 +195,24 @@ class TestDataset:
         assert (0x10, 0x5f) not in self.ds
         assert 'CommandGroupLength' in self.ds
         # Use a negative tag to cause an exception
-        assert (-0x0010, 0x0010) not in self.ds
+        msg = (
+            r"Invalid value used with the 'in' operator: must "
+            r"be an element tag as a 2-tuple or int, or an element keyword"
+        )
+        with pytest.raises(ValueError, match=msg):
+            (-0x0010, 0x0010) in self.ds
+
         # Random non-existent property
-        assert 'random name' not in self.ds
+        with pytest.raises(ValueError, match=msg):
+            assert 'random name' in self.ds
+
+        # Overflowing tag
+        msg = (
+            r"Invalid element tag value: tags have a maximum value of "
+            r"\(0xFFFF, 0xFFFF\)"
+        )
+        with pytest.raises(OverflowError, match=msg):
+            0x100100010 in self.ds
 
     def test_clear(self):
         assert 1 == len(self.ds)
@@ -278,8 +306,11 @@ class TestDataset:
         assert 2 == len(self.ds)
 
     def test_setdefault_invalid_keyword(self):
-        with pytest.raises(ValueError, match="'FooBar' is not a valid int or"
-                                             " DICOM keyword"):
+        msg = (
+            r"Invalid value used with the 'in' operator: must "
+            r"be an element tag as a 2-tuple or int, or an element keyword"
+        )
+        with pytest.raises(ValueError, match=msg):
             self.ds.setdefault('FooBar', 'foo')
 
     def test_get_exists1(self):
@@ -372,12 +403,17 @@ class TestDataset:
         assert callable(ds.group_dataset)
         assert 'group_dataset' in dir(ds)
 
-    def test_dir(self):
+    def test_dir(self, setattr_warn):
         """Dataset.dir() returns sorted list of named data_elements."""
         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',
@@ -386,12 +422,17 @@ class TestDataset:
                     'XRayTubeCurrent']
         assert expected == ds.dir()
 
-    def test_dir_filter(self):
+    def test_dir_filter(self, setattr_warn):
         """Test Dataset.dir(*filters) works OK."""
         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
@@ -542,15 +583,21 @@ class TestDataset:
         # Make sure Dataset.__eq__() is being used, not dict__eq__()
         assert not d == {'SOPInstanceUID': '1.2.3.4'}
 
-    def test_equality_unknown(self):
+    def test_equality_unknown(self, setattr_warn):
         """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):
@@ -1563,7 +1610,7 @@ class TestDatasetElements:
         assert ExplicitVRBigEndian == self.ds.file_meta.TransferSyntaxUID
 
         assert 'MediaStorageSOPClassUID' not in self.ds.file_meta
-        assert 'MediaStorageSOPInstanceUID ' not in self.ds.file_meta
+        assert 'MediaStorageSOPInstanceUID' not in self.ds.file_meta
         with pytest.raises(ValueError,
                            match='Missing required File Meta .*'):
             self.ds.fix_meta_info(enforce_standard=True)
@@ -1896,3 +1943,113 @@ class TestFileMeta:
         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."""
+    original = config.INVALID_KEYWORD_BEHAVIOR
+    config.INVALID_KEYWORD_BEHAVIOR = "ERROR"
+    yield
+    config.INVALID_KEYWORD_BEHAVIOR = original
+
+
+@pytest.fixture
+def setattr_ignore():
+    """Ignore Dataset.__setattr__() close keyword matches."""
+    original = config.INVALID_KEYWORD_BEHAVIOR
+    config.INVALID_KEYWORD_BEHAVIOR = "IGNORE"
+    yield
+    config.INVALID_KEYWORD_BEHAVIOR = original
+
+
+@pytest.fixture
+def setattr_warn():
+    """Warn on Dataset.__setattr__() close keyword matches."""
+    original = config.INVALID_KEYWORD_BEHAVIOR
+    config.INVALID_KEYWORD_BEHAVIOR = "WARN"
+    yield
+    config.INVALID_KEYWORD_BEHAVIOR = original
+
+
+def test_setattr_warns(setattr_warn):
+    """"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_ignore(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
diff --git a/pydicom/tests/test_util.py b/pydicom/tests/test_util.py
index 20b7b40aa..263899b45 100644
--- a/pydicom/tests/test_util.py
+++ b/pydicom/tests/test_util.py
@@ -117,16 +117,18 @@ class TestCodify:
         # ControlPointSequence
         elem = DataElement(0x300A0111, 'SQ', [])
         elem.value.append(Dataset())
-        elem[0].PatientID = '1234'
-        out = "\n"
-        out += "# Control Point Sequence\n"
-        out += "cp_sequence = Sequence()\n"
-        out += "ds.ControlPointSequence = cp_sequence\n"
-        out += "\n"
-        out += "# Control Point Sequence: Control Point 1\n"
-        out += "cp1 = Dataset()\n"
-        out += "cp1.PatientID = '1234'\n"
-        out += "cp_sequence.append(cp1)"
+        elem.value[0].PatientID = '1234'
+        out = (
+            "\n"
+            "# Control Point Sequence\n"
+            "cp_sequence = Sequence()\n"
+            "ds.ControlPointSequence = cp_sequence\n"
+            "\n"
+            "# Control Point Sequence: Control Point 1\n"
+            "cp1 = Dataset()\n"
+            "cp1.PatientID = '1234'\n"
+            "cp_sequence.append(cp1)"
+        )
 
         assert out == code_dataelem(elem)