search

pydicom / deid

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

Error when using functions #132

Closed Mattertorian closed 4 years ago

Mattertorian commented 4 years ago

Thank you for this nice library!

Currently, I experience an error, when I try to use a function in my deid recipe.

I have tried to follow the pusheenize example but still experience the issue.

I get the TypeError: "pusheenize() got multiple values for argument 'item'"

Poking around the source code, I can't see any reasons for this error.

Please tell me if I am making any obvious mistakes.

Thank you!

version: 0.2.1 python: `from deid.config import DeidRecipe from deid.dicom import get_files from deid.data import get_dataset from pathlib import Path

def pusheenize(item, value, field, dicom): value = item.get(str(field), '') if "Name" in str(field): value = "Pusheena" + value.replace(' ', '') return value

deid_path = Path('C:/', 'Users', 'MBRE0082', 'Desktop', 'deid_recipes', 'deid.dicom-pusheen') recipe = DeidRecipe(deid_path)

dicom_files = list(get_files(get_dataset('dicom-cookies')))

from deid.dicom import get_identifiers items = get_identifiers(dicom_files)

updated_items = dict()

for key, item in items.items(): updated_items[key] = item updated_items[key]['pusheenize'] = pusheenize

from deid.dicom import replace_identifiers cleaned_files = replace_identifiers(dicom_files=dicom_files, deid=recipe, ids=updated_items)

print(cleaned_files[0])`

recipe: ´FORMAT dicom

%header

REMOVE all func:pusheenize´

vsoch commented 4 years ago

Taking a look!

vsoch commented 4 years ago

Okay I found the bug - and the example seems off too, because why would we want to create a function to replace values with something pusheen related but then use REMOVE (which expects boolean?)

vsoch commented 4 years ago

It looks like the recipe in the docs does use replace, e.g.,

FORMAT dicom

%header

REPLACE all func:pusheenize
Mattertorian commented 4 years ago

Dang. I changed to REMOVE to check the error also happened in that case. Seems I forgot to revert it before copy-pasting the code here. Unfortunately, I still get the same error with REPLACE.

vsoch commented 4 years ago

yep! I've found the error and fixed it - but if you use REMOVE it won't return the expected result (so I was thrown off!) I'm still testing locally but should have a PR soon.

vsoch commented 4 years ago

okay please review https://github.com/pydicom/deid/pull/133

Mattertorian commented 4 years ago

Those changes fixed it for me! Thank you very much for the quick response!

I think this issue can be close with your pull request.

One more point: right now a long list of skipped tags is printed. Is there a way to avoid this printing?

Thanks again. I look forward to using the library!

vsoch commented 4 years ago

That's based on the logger level, so (depending on your interaction) you can usually export MESSAGELEVEL=QUIET (or some other preferred level).

vsoch commented 4 years ago

Thanks for reporting the issue! We had a major refactor of the dicom parser so I suspect there are issues hiding out there like this! This is just released:

https://pypi.org/project/deid/0.2.11/

And please report anything else that might come up so I can address promptly.