Skip to content
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

Series filtering out based on description string #351

Merged
merged 34 commits into from
Apr 10, 2024

Conversation

jeremyestein
Copy link
Contributor

@jeremyestein jeremyestein commented Mar 21, 2024

Fixes #346

Apply series filtering in orthanc anon, just before anonymisation. So if series filtering settings change, we can re-run without having to re-download everything to orthanc raw.

TODO

  • System tests
    • In _check_dcm_tags_from_zip, need to set the expected study/series/instance structure (see _populate_vna for the source data)
    • Explain why unhashed AccNum and PatientID ended up in orthanc-anon(!) Might be due to throwing in the received instance callback. Investigate separately? (see orthanc-anon ReceivedInstanceCallback must not allow exceptions to go uncaught or anonymisation will be compromised #368 )
    • Fix whatever is causing instances in the same study to not end up in the same zip file - The in-test generated dicom data is not mixing well with the static dicom files, so they are not recognised as the same study.
    • Decide if we need anything more complicated than series exclusion based on simple substring matches (eg. an include list, regexes...)

Approaches considered but not implemented

  • Download study to orthanc raw as before, then delete series/instances that we don't want using the API
  • Query VNA (without /retrieve) for series matching criteria, locally filter, and then instead of downloading studies as we do now, query (with /retrieve) for series IDs that we do want

Problems with the above:

  • Would need to requery VNA if series filtering criteria changed
  • Would need to keep track of downloads in the DB at a series level. Would CLI know what to re-request?
  • Some series level operations not even possible?

@jeremyestein jeremyestein marked this pull request as ready for review March 27, 2024 16:39
@stefpiatek
Copy link
Contributor

Explain why unhashed AccNum and PatientID ended up in orthanc-anon(!) Might be due to throwing in the received instance callback. Investigate separately?

I think create a separate issue. Probably worth querying for the pseudonymised study ID in PIXL DB before exporting, then updating the ORM instance with the export date after the export? That way if the pseudonymised ID hasn't been added, it won't be found in the database and an exception will be thrown and it won't be exported

Fix whatever is causing instances in the same study to not end up in the same zip file - The in-test generated dicom data is not mixing well with the static dicom files, so they are not recognised as the same study.

IIRC for system tests we decreased the waiting time for a study to be taken as stable, might be that? Or are they seemingly a different study altogether?

Copy link
Contributor

@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks Jeremy!

orthanc/orthanc-raw/plugin/pixl.py Outdated Show resolved Hide resolved
pixl_dcmd/src/pixl_dcmd/main.py Outdated Show resolved Hide resolved
pixl_dcmd/tests/test_main.py Outdated Show resolved Hide resolved
pixl_imaging/src/pixl_imaging/_orthanc.py Show resolved Hide resolved
test/system_test.py Outdated Show resolved Hide resolved
@jeremyestein jeremyestein merged commit 20fb45d into main Apr 10, 2024
8 checks passed
@jeremyestein jeremyestein deleted the jeremy/series-filtering branch April 10, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter out unneeded series from studies
2 participants