Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checks that values satisfy requirements of the VR #136

Open
CPBridge opened this issue Nov 16, 2021 · 3 comments
Open

Checks that values satisfy requirements of the VR #136

CPBridge opened this issue Nov 16, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@CPBridge
Copy link
Collaborator

This has been discussed before (e.g. here) but we should have an issue to track it.

Pydicom is very loose in what it allows you to set as an attribute's value, even when you have the global configuration option pydicom.config.enforce_valid_values set to True. We have previously encountered and resolved this narrowly for decimal strings (DS) #57 #65, but the issue is broader. Checks for the other VRs are largely absent from pydicom, but many VRs have limits on length of the string, list of allowable characters, capitalisation, etc (see standard). The result is many one-off checks being including to check user-supplied values in highdicom, as well as probably many missed checks that could allow files with invalid values to be produced. We should tackle this in a more unified way to reduce redundancy and probability of invalid values slipping through the net.

My feeling is that as far as possible we should add this functionality to pydicom and then integrate into highdicom.

@CPBridge CPBridge added the enhancement New feature or request label Nov 16, 2021
@CPBridge CPBridge self-assigned this Nov 16, 2021
@hackermd
Copy link
Collaborator

See also pydicom/pydicom#1414

@hackermd
Copy link
Collaborator

We should avoid the global configuration as much as possible. My preferred approach would be to add a parameter to the constructor of Dataset and friends (Sequence, etc.) such as enforce_valid_vr. That should allow us to enforce correct encoding when creating new objects while potentially allowing some invalid values when reading existing objects.

@CPBridge
Copy link
Collaborator Author

CPBridge commented Apr 2, 2022

A note here that pydicom 2.3.0 has a lot of new functionality to help us here, we just need to figure out how to best make use of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants