Force conformance on ACR-NEMA 300 formatted DA values

[scaramallion]

We have code in the DA class that allow invalidly formatted values:

>>> from pydicom.valuerep import DA
>>> DA('2000.01.01')
"2000.01.01"

I suggest we force conformance on these values (just when the DA class is being used, not when using str) and warn instead of allowing the pass-through:

>>> DA("2000.01.01")
UserWarning: Invalid DA value, will be forced to etc, etc...
"20000101"

[mrbean-bremen]

I was actually thinking of converting this old data by just using the canonical string representation as original_string (did so in the first version of the PR, but reverted that) - I could still do this in the PR. Not sure about the warning, but it probably makes sense. @darcymason , what do you think?

[darcymason]

I was actually thinking of converting this old data by just using the canonical string representation as original_string (did so in the first version of the PR, but reverted that) - I could still do this in the PR. Not sure about the warning, but it probably makes sense. @darcymason , what do you think?

I'm not sure what you mean by "canonical" here - conforming to the Standard? At a quick glance at the first version of PR, it looks like it kept the original.

In any case I think original_string string should always mean that - the original string that was set. That is useful for round-trip testing, or for people who want to tweak one or two data elements in a file but leave everything else alone.

On writing, it could be converted to Standard-conformant if enforce_valid_values is set.

On the other hand, I'd support warning on setting as @scaramallion suggested, and could extend that to all invalid DICOM values. Do we have a flag to turn warnings like that on or off? It feels like we should but I can't think of an existing one that fits. It would be good to have it for people working with old files not to be bothered with too many warning messages.

I just glanced at the pydicom config flags - we do have a lot of them, and maybe some consolidation would help - this goes to comments in #1232 about taming the idea of 'strict' DICOM flags.

Apologies that the last couple of paragraphs perhaps adding more to this than just the discussion at hand. But perhaps it is worth thinking about that larger picture a little while we are on the subject.

[mrbean-bremen]

Thank you for the extensive response!

I'm not sure what you mean by "canonical" here - conforming to the Standard?

Yes, should have called it that...

At a quick glance at the first version of PR, it looks like it kept the original.

I didn't push that first version - I was a bit undecided, but in the end thought that original_string shall be that - the original string, the same reason you also state.

On writing, it could be converted to Standard-conformant if enforce_valid_values is set.

Agreed about the writing. I actually think that writing conforming to the Standard should be the default behavior.
We currently use enforce_valid_values only for reading (e.g. raise an exception instead of just warning on invalid DICOM). The meaning for writing would be a bit different - maybe we want to separate this. Though this would probably be a part of the mentioned configuration consolidation, maybe we should discuss that one in a separate issue.

Anyway, I think I will finish the PR as is a minor change, and we can take the time to discuss this issue in more depth.

[darcymason]

Though this would probably be a part of the mentioned configuration consolidation, maybe we should discuss that one in a separate issue.

That is happening in #1232 now...