-
-
Notifications
You must be signed in to change notification settings - Fork 48
Specify more than one coordinates in the filter #230
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
Comments
Here is an example of an entry with two coordinates: https://github.com/pydicom/deid/blob/master/deid/data/deid.dicom#L301-L308 |
I tried that but It's not working. Only the First coordinates were removed. |
That's a bug then! Are you able to share some dummy data so I can use for a debuggig/test case? |
Actually I have a good test case already! I should be able to work on this and propose a fix today - stay tuned! |
I don't have any dummy data. Sorry for that. Thanks |
@NirutaDhimal I tested the syntax with multiple coordinates and it appears to work as I'd expect (both sets show up):
I think very likely the issue is that you either have a bug in the bounding box you are asking for, or a "contains" statement that doesn't apply to your image. It's hard to debug beyond that without seeing your data. However, the format for the coordinates that I showed above is correct. |
@vsoch I printed client.detect() result and both set shows up in the coordinates. But when I did client.save(), the saved image still contains the text in the 2nd coordinate. |
Did you do |
Yes, I did that. First I did client.detect() and then client.clean() and at last I did client.save() |
I could help to debug but I’d need you to send me a dummy example (an image and recipe) that reproduces the error. Just to confirm - this is a 2D dicom? perhaps you could take your image, make a copy that replaces identifiers, write all pixels as some single value (e.g the mean across the image) and then we would try to clean and write a different value? |
Yes its a 2D dicom file and it has 3 channels. I will send you the dummy data |
@vsoch This link has the dummy data and the deid.dicom file. Please find it. I have created the dummy file by the process you described above. |
Thank you @NirutaDhimal ! It's after midnight here, but I should be able to debug this (and get back to you) later tomorrow. Stay tuned! |
okay I think I see the cleaning is working? Are you saving? I am using your files in my PWD. from deid.dicom import DicomCleaner
import os
client = DicomCleaner(deid='deid.dicom')
client = DicomCleaner(output_folder=os.getcwd())
client.detect('dummy.dcm') {'flagged': True,
'results': [{'reason': 'and Modality contains CT Manufacturer contains GE Rows equals 1024 BurnedInAnnotation contains YES',
'group': 'graylist',
'coordinates': [[0, '0,0,300,150'], [0, '724,0,300,150']]},
{'reason': 'and Manufacturer contains GE Modality contains CT ManufacturerModelName contains GE|Stanford SeriesDescription contains Dose or ImageType contains SCREEN SAVE',
'group': 'graylist',
'coordinates': [[0, '0,0,512,121']]},
{'reason': 'and Modality contains CT|MR ImageType contains DERIVED|SECONDARY|SCREEN|SAVE',
'group': 'blacklist',
'coordinates': []}]} client.clean()
# This is needed to save the cleaned image - the original won't be touched
client.save_dicom() And then the image I see there is a clear change. Here is the original: And changed:
|
@vsoch only one coordinates were removed from the dummy example when I tried it. |
Something is different between what you and I are doing because my results lists has three entries, and yours has one. Make sure you are using the master branch of deid and copying what I did exactly, and with the same file. |
I installed deid using pip and I am working on the same file. I try to copy what you exactly did |
Could it be that I used the default deid.dicom config and it covers the locations? Or should it have covered the top left? |
Given your coordinates, it should have covered the top left and top right. Which deid.dicom file did you use? I haven't specified any group for this in the deid.dicom {'reason': 'and Manufacturer contains GE Modality contains CT ManufacturerModelName contains GE|Stanford SeriesDescription contains Dose or ImageType contains SCREEN SAVE', |
Yes! So if the user doesn’t provide a custom recipe, we use this default, which is packaged with deid in the data folder. It seems that the headers in your dicom file triggered some of the default locations, but perhaps we are missing the top left? Should we try adding it? And where did you derive your metadata and coordinates from? |
It's supposed to remove on top left and top right |
Ah ok so our viewers are flipped! What I'm seeing is that:
Now that I understand there should be another box on the other side, let me take a look again at the dummy data you sent and see if I can make sense of it. Back in a bit! |
ok thanks for helping me |
I used weasis viewer. |
Quick question - how did you derive the second coordinate? The image is 1024 by 1024, and the coordinate format is two mins followed by two maxes, so if you expect it to blank out the top right I would have expected the mins to be smaller than the maxes. What appears to be happening is that you are "selecting" a box of empty pixels. |
Okay so here is an example where I've adjusted your coordinates. There is probably a better way to describe it, but what I'm doing is shuffling around your coordinates so min values < max values, and extending the box to span your image (and remember my viewer is flipped). Trying to put this into human terms, I changed your recipe to: FORMAT dicom
%filter graylist
LABEL ANNOTATIONS
contains Modality CT
+ contains ConversionType WSD
+ contains Manufacturer GE
+ contains ImageType MIP
+ contains ManufacturerModelName Revolution
+ contains SoftwareVersions taix_bj.84.i386
+ contains Rows 1024
+ contains Columns 1024
coordinates 1,12,227,195
coordinates 271,9,1024,118
# minr, minc, maxr, maxc
# indexed in the data like mask[minc:maxc, minr:maxr]
# those values are set to 0 to "blank" them
%header
ADD PatientIdentityRemoved YES
REPLACE PatientID var:id
REPLACE SOPInstanceUID var:source_id So from the above, if either of the max values is less than a min, that's an index that will silently error (giving you an empty array). So for example, if I adjust your values to the above I get: So from what I can tell, the issue here is that the coordinate is wrong. The only reason we got two boxes on my first try is that I erroneously used the default deid.dicom recipe, and not the one you supplied. So if you derived these on your own, make sure they are in that format I've shown above, and that supplying the values actually produces boxes (the first one did not, the second box was an empty array). |
coordinates are in this format (x,y,w,h) right? My second ccordinate is (737, 9, 271, 118) which means the bottom right of the bounding box is has pixel (1108,127) both of which are less than 1024 |
We follow the same convention as CTP: https://mircwiki.rsna.org/index.php?title=The_CTP_DICOM_Pixel_Anonymizer and you can see our original discussion here: https://gist.github.com/vsoch/df6957be12c34e62b21000603f1687e5 and documentation of it here: deid/deid/dicom/pixels/detect.py Lines 271 to 282 in 27d46bb
Based on this logic, you have an index Pinging @fimafurman and @wetzelj in case I'm missing something - I've been away from imaging a long time and don't trust myself. |
Perfect! Let's wait to hear from @wetzelj and @fimafurman before closing the issue! If we somehow used a modified format (and also CTP coordinates) we will likely want to tweak one or the other. For the time being, if this works correctly use the format we currently have. Thanks @NirutaDhimal ! |
Agree! So what about if we allow both - we have a "ctp" type of coordinate (that gets read differently) and we maintain the default (to not break people's workflows?) Or do you think it's better to have one standard, and fix up everything we do now to conform to that? |
Personally I think a new "ctp" type would be preferred over a "one standard". To flip from our current pattern to the "ctp" would introduce a subtle breaking change - that I could envision people could miss very easily since it's just a behavior change. If we did want to force a single standard, I would prefer to introduce ctp as a new type and then, if we really want it, at a later point in time deprecate the "coordinates" method. |
Gotcha! The trick then will be figuring out which coordinate come from CTP. I can take a first pass at this if y'all are good with this approach. |
okay please see #234 |
We just merged the fix that should resolve this - thanks everyone! |
I am trying to remove clean pixels of 2 -3 coordinates of a dicom file. How do I specify it in the filter ?

[
This is my filter But I want to clean pixels on bounding box 4,462,23,102. How do I add this in filter?
The text was updated successfully, but these errors were encountered: