UCLH-Foundry / PIXL

PIXL Image eXtraction Laboratory
Apache License 2.0
8 stars 0 forks source link

Embed project name as tag in DICOM image #329

Closed jeremyestein closed 5 months ago

jeremyestein commented 5 months ago

Fix #296

TODO:

jeremyestein commented 5 months ago

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)

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.

jeremyestein commented 5 months ago

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 get_project_slug_from_hashid, as a side effect it checks whether the image has already been exported. Do we want to keep a call to _query_existing_image for this reason? Or will the uploader service fix this better anyway, and we can live with re-uploads for the time being? I'm not sure in what situations we would encounter this condition.

    with PixlSession() as pixl_session, pixl_session.begin():
        existing_image = _query_existing_image(pixl_session, hashed_value)

        if existing_image.exported_at is not None:
            msg = "Image already exported"
            raise RuntimeError(msg)