search

pydicom / deid

best effort anonymization for medical images using python
https://pydicom.github.io/deid/
MIT License
140 stars 43 forks source link

Empty values within FIELDS identified in %values sections #144

Closed wetzelj closed 4 years ago

wetzelj commented 4 years ago

This bug presents itself in two locations:

  1. When a %values lists contains a FIELD which has an empty value, the empty string is used to search for fields on which to act in the action. This causes all fields to be identified and all header fields to be removed.
  2. When a %values lists contains only FIELDS/SPLITS which result in empty values within the values list, the empty list is used to when searching for fields to target and results in the removal of all header fields.

Values being added to a %values list should be interrogated so that empty strings are not added to values lists and %values lists should be interrogated so that if a values list contains no values, the action is ignored.

I've put together a possible fix as well as unit tests for this condition within my fix/empty_values_lists branch. Let me know what you think.

wetzelj commented 4 years ago

FYI... I recently updated to black v20.8b1. It appears to have introduced some changes to the docstring quotes. If there's a specific version I should be using, let me know. :)

vsoch commented 4 years ago

Your branch looks great! One suggestion, it's slightly more python to do:

if values:

instead of

if len(values) > 0:

For black we install the latest version, so it might be okay (we can see how it runs when the PR is opened). We can pin a version if necessary after that. Open away! :)

vsoch commented 4 years ago

Thank you again for this! It’s really lovely to have someone to collaborate with, and fulfilling to see the library getting better.