Pydicom: Print a warning when assigning a non-existing DICOM Tag

[razorx89]

Is your feature request related to a problem? Please describe.
This problem is related to #1013

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.

[mrbean-bremen]

Hm, generally you can always add an attribute this way in Python, so forbidding this could be a bit unexpected, especially if working with classes derived from DataSet. Anyway this is a valid request, in my opinion. @darcymason, what do you think?

[razorx89]

Maybe it could be restricted to names which are pascal-case, since those don't follow the PEP naming conventions.

[mrbean-bremen]

Yes, that could indeed be a good compromise.

[scaramallion]

Maybe yet another config option. I don't really like the idea of forcing people to adapt their usual conventions. Although I have been bitten by this a couple of times...

[darcymason]

I was just thinking of this again recently also, and occasionally get bitten momentarily.

I support the idea of warning on non-DICOM CamelCase attributes for now. I believe it is rare that people would intentionally use their own attributes, and even less likely they would use CamelCase if they did, and it would only warn for now, so is not stopping their code from operating normally. In pydicom 3.x, I think it should raise an exception rather than a warning and then maybe it should be able to be turned off.

How about this: a config option (allowed values of 'off', 'warn' or 'error') for non-dicom attributes CamelCase. It defaults now to 'warn' in pydicom 2.x but 'error' in 3.x. That gives people the option to turn it off in 3.x if it causes trouble with legacy code.

Or, maybe there could be a more strict value, to warn on _any_ attribute setting that is not a name in a list: something like Python's __slots__ concept. But we could add that later.

One caveat: I think warnings are by default only issued once now; this one should probably warn on each occasion if possible, otherwise typos could be silently missed.

And ... side thought ... maybe we should have some option for people to turn on all future config defaults -- allows people to ensure code will work for next major pydicom version. Somewhat like Python's command-line options to turn all warnings into errors.

[SimonBiggs]

maybe we should have some option for people to turn on all future config defaults -- allows people to ensure code will work for next major _pydicom_ version. Somewhat like Python's command-line options to turn all warnings into errors.

I really like that idea (and would use it immediately). Having an error raised for non-dicom tags is also a config option I would set to error in my own code if the option was available.

Actually... Past me liked the idea so much... I think I turned it on already:

https://github.com/pymedphys/pymedphys/blob/c3052444e00062d0b3e7a4ffdafe40a7c756d523/pymedphys/_dicom/create.py#L52

[darcymason]

Another thought -- the Datasets.__contains__ should also check that the keyword is a valid DICOM keyword, otherwise it will definitely return False for a typo, or if the keyword is new since the pydicom dictionary was last updated.

[scaramallion]

This has come in handy already:

../pydicom/dataset.py:1891: UserWarning: Camel case attribute 'PatientBirtDate' used which is not in the element keyword data dictionary