-
Notifications
You must be signed in to change notification settings - Fork 1
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
Embed project name as tag in DICOM image #329
Conversation
Also add to system test to check presence of private tag. Think this will have to be moved to check in a different place but has value for checking other pseudonymised tags?
projects/configs/tag-operations/test-extract-uclh-omop-cdm.yaml
Outdated
Show resolved
Hide resolved
due to never changing this via the API during anonymisation.
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.
Looking good! 🚀
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.
We also need to use the project name from within orthanc anon (IIRC code where we do this is in pixl_dcmd), instead of querying the database to get the project name (see the checklist in the issue)
in a private tag, and no longer from the PIXL database.
This is fixed now. However, it's not ideal that the private tag info (0x000B, 0x01, "UCLH PIXL", "UCLHPIXLProjectName") is copied over so many locations in python, json, and yaml. The python locations could surely be easily reduced to one place, but the others may be a bit more fiddly. One approach would be to have a single source of truth for our private tags in a json file, and use jsonnet to generate the various json config files from this. Python can trivially load the json file. For the yaml I'm not sure - yaml is a superset of JSON so maybe when loading the tag scheme we can concatenate it with the json file? Or just switch the tag schemes to json(net)? Anyway, this should go in a separate issue. But I'll try and fix the python usages in this PR. |
…tag. There is still duplication in json and yaml files unfortunately.
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.
Looks good, thanks Jeremy!
Can we also remove the use of get_project_slug_from_hashid
from orthanc-anon/pixl.py, instead querying the API for the project name using the resourceId
?, then we can pass that to the ftp upload, and then we don't have to query for the project name in core.uploader._ftps
and then remove the get_project_slug_from_hashid
entirely
hashed_patient_id = _get_patient_id(resourceId)
project_slug = get_project_slug_from_hashid(hashed_patient_id)
Looking at
|
…he local orthanc API which we're calling anyway. orthanc anon does need the dictionary so we can refer to the private tag by name in the API.
collisions, which orthanc can't handle
cause us problems.
image has already been uploaded. Clarify the PatientID tag meaning.
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.
Great, thanks for finishing this up Jeremy!
Fix #296
TODO: