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

dicom-anonymiser Generic Wrapper Microservice #1679

Merged
merged 59 commits into from
Jun 13, 2024
Merged

Conversation

darshad-github
Copy link
Contributor

@darshad-github darshad-github commented Nov 22, 2023

Proposed Changes

This pull request is initiated to review development progress against gitlab issue #137 which is to design a C# generic wrapper microservice that can call external scripts to anonymise DICOM files.

Proposed Changes (06-02-2024):

As a reusable component, this microservice takes a message, performs CTP anonymisation followed by SR anonymisation or DICOM anonymisation on the dicom file based on the image modality. It creates a new anonymised file, and finally generates a message containing the new filename. Additionally, the microservice is able to handle the failure of the script.

  • Branch: feature/dicom-anonymiser
    • Renamed existing class DicomAnonymiser to DefaultAnonymiser that implements the IDicomAnonymiser interface responsible for anonymising DICOM files.
    • Modified the Anonymise method across 3 different anonymisation methods (CreateCTPProcess, CreateDICOMProcess, CreateSRProcess) which creates the required process based on different configs.
    • Add a new method called CreateDICOMToTextProcess responsible for converting dicom files to text required by CreateSRProcess.
    • Modified the configuration file, DicomAnonymiserConfigs.json with additional paths required for setting up CTP and SR anonymisation process.
    • Modified the DicomAnonymiserConsumer class to include a DicomFile object using the FellowOakDicom package to extract message modality.
    • Updated the GlobalOptions.cs class with the required dicom anonymiser configurations and removed DicomAnonymiserConfigs.json altogether.

Proposed Changes (22-11-2023):

As a reusable component, this microservice takes a message, performs pixel data anonymisation on the dicom file, creates a new anonymised file, and finally generates a message containing the new filename. Additionally, the microservice is able to handle the failure of the script.

  • Branch: feature/dicom-anonymiser
    • Introduced a new class DicomAnonymiser that implements the IDicomAnonymiser interface responsible for anonymising DICOM files.
    • Modified the Anonymise method in the IDicomAnonymiser interface to include an additional ExtractFileMessage parameter.
    • Added the DicomAnonymiser value to the AnonymiserType enum to represent the DICOM file anonymiser.
    • Updated the AnonymiserFactory to create a new instance of DicomAnonymiser when the DicomAnonymiser type is specified.
    • Added a new configuration file, DicomAnonymiserConfigs.json which contains paths setting up the anonymisation process.
    • Updated the DicomAnonymiserConsumer class to use the DicomAnonymiser for anonymising files.

Service Documentation:

Types of changes

What types of changes does your code introduce? Tick all that apply.

  • Bugfix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation-Only Update (if none of the other choices apply)
    • In this case, ensure that the message of the head commit from the source branch is prefixed with [skip ci]

Checklist

By opening this PR, I confirm that I have:

  • Reviewed the contributing guidelines for this repository
  • Ensured that the PR branch is in sync with the target branch (i.e. it is automatically merge-able)
  • Updated any relevant API documentation
  • Created or updated any tests if relevant
  • Created a news file
    • NOTE: This must include any changes to any of the following files: default.yaml, any of the RabbitMQ server configurations, GlobalOptions.cs
  • Listed myself in the CONTRIBUTORS file 🚀
  • Requested a review by one of the repository maintainers

Issues

If relevant, tag any issues that are expected to be resolved with this PR. E.g.:

This commit introduces a new class, DicomAnonymiser, which implements the IDicomAnonymiser interface. This class is responsible for anonymising DICOM files, a common format for medical imaging data.

The DicomAnonymiser class uses a configuration file to set up paths for various resources. It then uses these paths to create a process that runs a shell script for anonymising the DICOM files. The class also includes error handling to ensure that any issues during the anonymisation process are logged and appropriately handled.
…eter

This commit modifies the Anonymise method in the IDicomAnonymiser interface to include an additional ExtractFileMessage parameter. This change allows the method to use the message's modality when anonymising the specified source file to the destination file.
This commit adds the DicomAnonymiser value to the AnonymiserType enum. This value represents the DICOM file anonymiser and is assigned the integer value of 1. This allows for clear and type-safe references to the DICOM anonymiser throughout the codebase.
This commit updates the AnonymiserFactory to create a new instance of DicomAnonymiser when the DicomAnonymiser type is specified. This allows the factory to produce the correct anonymiser based on the provided AnonymiserType.
This commit updates the DicomAnonymiserConsumer class to use the DicomAnonymiser for anonymising files. The Anonymise method is called with the message, source file, and destination file as parameters. This change integrates the DicomAnonymiser into the consumer's workflow.
This commit adds a new configuration file, DicomAnonymiserConfigs.json. This file contains paths for the virtual environment, shell script, DICOM pixel anonymiser, and SMI services. These paths are used by the DicomAnonymiser class to set up the necessary resources for anonymising DICOM files.
@rkm rkm marked this pull request as draft December 13, 2023 15:12
Copy link
Member

@rkm rkm left a comment

Choose a reason for hiding this comment

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

Still a little early in development for a full review, but I've left one comment in the meantime. Happy to discuss working in the GlobalOptions class and working through some of the TODOs!

@darshad-github darshad-github requested a review from rkm February 9, 2024 14:07
Copy link

codecov bot commented Mar 29, 2024

Codecov Report

Attention: Patch coverage is 23.80952% with 64 lines in your changes missing coverage. Please review.

Project coverage is 59.19%. Comparing base (c0038ad) to head (392eee2).

Files Patch % Lines
...s.DicomAnonymiser/Anonymisers/DefaultAnonymiser.cs 0.00% 54 Missing ⚠️
...ervices.DicomAnonymiser/DicomAnonymiserConsumer.cs 61.90% 6 Missing and 2 partials ⚠️
...s.DicomAnonymiser/Anonymisers/AnonymiserFactory.cs 50.00% 1 Missing ⚠️
...croservices.DicomAnonymiser/DicomAnonymiserHost.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1679      +/-   ##
==========================================
- Coverage   59.82%   59.19%   -0.63%     
==========================================
  Files         176      177       +1     
  Lines        6312     6391      +79     
  Branches      861      877      +16     
==========================================
+ Hits         3776     3783       +7     
- Misses       2294     2363      +69     
- Partials      242      245       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rkm rkm merged commit 71b5147 into main Jun 13, 2024
17 of 19 checks passed
@rkm rkm deleted the feature/dicom-anonymiser branch June 13, 2024 19:30
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.

2 participants