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

Is deid.dicom aquivalent to CTP's pixel.anonymizer.script? #63

Closed
fimafurman opened this issue Sep 12, 2018 · 13 comments · Fixed by #80
Closed

Is deid.dicom aquivalent to CTP's pixel.anonymizer.script? #63

fimafurman opened this issue Sep 12, 2018 · 13 comments · Fixed by #80

Comments

@fimafurman
Copy link

I noticed that deid.dicom is missing entries in CTP's pixel.anonymizer.script - is deid's deid.dicom config file modeled after CTP's script?

@vsoch
Copy link
Member

vsoch commented Sep 12, 2018

Yes! Would you care to PR to update? Would be greatly appreciated!

@fimafurman
Copy link
Author

fimafurman commented Sep 12, 2018 via email

@fimafurman
Copy link
Author

Question:

I have an example image which gets both blacklisted and greylisted with coordinates. It seems to hit this blacklist test:

LABEL Burned In Annotation # (CTP)
contains ImageType SAVE

  • contains Modality CT|MR
    || contains SeriesDescription SAVE
    || contains BurnedInAnnotation YES
    || empty ImageType
    || empty DateOfSecondaryCapture
    || empty SecondaryCaptureDeviceManufacturer
    || empty SecondaryCaptureDeviceManufacturerModelName
    || empty SecondaryCaptureDeviceSoftwareVersions

With:

"group": "blacklist",
"reason": " ImageType contains SAVE and Modality contains CT|MR or SeriesDescription contains SAVE or BurnedInAnnotation contains YES or ImageType empty or DateOfSecondaryCapture empty or SecondaryCaptureDeviceManufacturer empty or SecondaryCaptureDeviceManufacturerModelName empty or SecondaryCaptureDeviceSoftwareVersions empty "

  1. Should the image be cleaned or deleted?
  2. Is there a way to group ad and or conditions in the script?

@vsoch
Copy link
Member

vsoch commented Sep 19, 2018

hey @fimafurman ! The idea of the recipe is that you are intended to write your own criteria to supplement or replace the default. There is no perfect default that everyone would be happy with. If you need to change the above, you can freely do so and distribute your own file, or if you think it should be default, do a PR to change the default! Now to answer your questions:

  • the blacklist is hit first, just by way of being higher in the file, since it's more "urgent" it takes priority (but it meets criteria for both of the groups as you can see) I don't see issue with this second point because it's reasonable that a user could want to have fuzzy categorization with respect to groups.
  • the image being cleaned or deleted is up to you - it's your tool / decision, etc.
  • the conditions are grouped based on the filter groups! So you can make your own group with a set of criteria! The names of "blacklist" and "greylist" etc are just arbitrary - you can call whatever you like.

@fimafurman
Copy link
Author

I would assume that it tries to mirror CTP functionality which does attempt to make a decision for you. So my question is more what is the default behavior with such condition if there are no override rules?

@vsoch
Copy link
Member

vsoch commented Sep 19, 2018

This default I had help developing with users that were actively invested in the filter criteria - honestly this is not something that I want to take responsibility for, and indeed I don't have the expertise. For this situation I would recommend that you figure out the filters that are missing / need to be changed, suggest fixes, and PR with them. If there are no "override rules" for something and it doesn't get flagged, it's not going to be flagged. I can't offer much more explanation than that - I'm not in the business of managing PHI or rules about them other than developing this initial default to really get users started, not enforce anything. Deid is not published or approved by any privacy board, so it's use at your own decision.

@fimafurman
Copy link
Author

Thanks - that's fair - I just wanted to understand a bit better how it behaves by default. I will play with it some more and based on testing, update the default recipe. Thanks!

@vsoch
Copy link
Member

vsoch commented Sep 19, 2018

Sounds good.

@vsoch
Copy link
Member

vsoch commented Dec 9, 2018

@fimafurman do you have any additions (logic for the parser) to contribute, or shall we close the issue?

@fimafurman
Copy link
Author

Yes. How would you like me to share it with you?

@vsoch
Copy link
Member

vsoch commented Dec 10, 2018

Any way that works for you! The easiest is likely dropping the file you are using here, and then I can open a PR to add to deid.

@fimafurman
Copy link
Author

fimafurman commented Dec 10, 2018

Attached below.

I was surprised how incomplete the CTP published file is, and, as the result the deid, so we had to populate file fairly manually. If you know of any sources of complete scripts for CTP (for example) I would appreciate it.
ffur_deid copy.dicom.zip

@vsoch
Copy link
Member

vsoch commented Dec 10, 2018

Excellent, thank you! I will get these integrated asap.

I share the same sentiment - I was really surprised that "industry standard" was lacking so badly. I think it must be the case that centers add their own custom logic and don't share, because the culture (I've noticed) is fairly closed off when it comes to sharing code, etc.

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

Successfully merging a pull request may close this issue.

2 participants