-
Notifications
You must be signed in to change notification settings - Fork 112
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
feat(anonymizer) - export Array tagNamesToEmpty and modify cleanTags #303
feat(anonymizer) - export Array tagNamesToEmpty and modify cleanTags #303
Conversation
…function cleanTags -> manage the tags to anonymize/modify in dicom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this 👍
…as optional arguments to cleanTags
Hello, |
Thanks for the contribution and the reminder 👍 |
@juliencastelneau it looks like there's at least one error due to the anonymizer - could you have a look? https://github.com/dcmjs-org/dcmjs/actions/runs/3047634658/jobs/4911834982#step:5:2486 |
The test fails because I added an argument to the method cleanTags
Add the lines below in anonymizer.test.js (line 26)
instead of
@pieper The best way is to modify the method cleanTags I guess. |
Thanks for looking into this. I think the second option is better since it maintains backwards compatibility for the API. We should also add a new test demonstrating the ways to use the method and confirm it keeps working. Yes, a new PR would be great since this has already been merged. |
For instance, a call to anonymizer should be: