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.

Sign up for GitHub

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

Jump to bottom

Add unit tests for anonymisation process #360

Merged
merged 15 commits into from
Apr 8, 2024
Merged

Add unit tests for anonymisation process #360

merged 15 commits into from
Apr 8, 2024

Conversation

milanmlft
Copy link
Member

@milanmlft milanmlft commented Mar 26, 2024 •
edited
Loading

Fixes #132

  • Refactor: move add_private_tag to core.dicom_tags
  • Only load project config when it's needed
  • Add note to refactor private tag addition in orthanc-raw
  • Add DICOM image test fixtures to test anonymisation
  • Add anonymisation unit tests

@milanmlft milanmlft changed the title Add unit test for anonymisation process WIP: Add unit test for anonymisation process Mar 26, 2024
@milanmlft milanmlft mentioned this pull request Mar 26, 2024
Merged
@milanmlft milanmlft marked this pull request as ready for review April 4, 2024 16:15
@milanmlft milanmlft requested a review from a team April 4, 2024 16:15
@milanmlft milanmlft changed the title WIP: Add unit test for anonymisation process Add unit tests for anonymisation process Apr 4, 2024
@milanmlft milanmlft mentioned this pull request Apr 4, 2024
Closed
peshence
peshence approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@peshence peshence left a comment

Choose a reason for hiding this comment

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

The whole private tag class is very odd to me, but that was already in there and everything else looks good. 🚀

@milanmlft
Copy link
Member Author

The whole private tag class is very odd to me, but that was already in there and everything else looks good. 🚀

What do you think is odd about it?
I guess it's just meant as a consistent interface to add custom private tags to DICOM datasets.

@peshence
Copy link
Contributor

peshence commented Apr 5, 2024

Yeah I had some misunderstandings about private tags that @HChughtai helped me out with so it makes a bit more sense now. Thanks both!

stefpiatek
stefpiatek approved these changes Apr 8, 2024
View reviewed changes
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.

Nice, thanks for getting this in

pixl_dcmd/tests/test_main.py Outdated Show resolved Hide resolved
@milanmlft @stefpiatek
Fix assertion
a52b47f 
Co-authored-by: Stef Piatek <s.piatek@ucl.ac.uk>
@milanmlft milanmlft enabled auto-merge (squash) April 8, 2024 10:05
@milanmlft milanmlft merged commit fc3fe93 into main Apr 8, 2024
8 checks passed
@milanmlft milanmlft deleted the milanmlft/132-unit-test-for-anonymisation-process branch April 8, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefpiatek stefpiatek stefpiatek approved these changes

@peshence peshence peshence approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unit test for anonymisation process
3 participants
@milanmlft @peshence @stefpiatek