Add configuration for DESPIAD project #572
Conversation
this is because resources sharing the same StudyInstanceUID in Orthanc Raw will be combined into a single resource in Orthanc Anon. Previously, we would try to export each resource after anonymisation, but this meant other resources sharing the same StudyInstanceUID were not exported.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #572 +/- ##
==========================================
- Coverage 87.51% 87.24% -0.28%
==========================================
Files 76 76
Lines 3437 3489 +52
==========================================
+ Hits 3008 3044 +36
- Misses 429 445 +16 ☔ View full report in Codecov by Sentry. |
p-j-smith
force-pushed
the
paul/despiad-config
branch
from
04532e7 to
5a47136
Compare
p-j-smith
force-pushed
the
paul/despiad-config
branch
from
784da58 to
28f92a6
Compare
In orthanc anon plugin, skip series that have few than min_instances_per_series instances
also default to allowing no manufacturers
pixl_dcmd/tests/test_main.py
Outdated
| @@ -194,7 +194,9 @@ def ids_for_parameterised_test(val: pathlib.Path) -> str: | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize( | |||
| ("yaml_file"), PROJECT_CONFIGS_DIR.glob("*.yaml"), ids=ids_for_parameterised_test | |||
| ("yaml_file"), | |||
| PROJECT_CONFIGS_DIR.glob("test-*.yaml"), | |||
There was a problem hiding this comment.
no longer testing anonymisation using the non-test project configs. If we want to test all project configs, we need to add:
allowed_manufacturers:
- regex: "^company"
exclude_series_numbers: []to all configs, unless there's a nicer way to do it?
There was a problem hiding this comment.
I think for the existing projects we can just allow all manufacturers, as that's what was implicitly done. We do have a test that ensures all configs are valid though right?
There was a problem hiding this comment.
ah yeah that's fair, and for DESPIAD shall we limit to known manufacturers? And yeah we have a separate test to check configs are valid
There was a problem hiding this comment.
I've allowed all manufacturers for the existing configs, and added them back to the test_anonymise_and_validate_dicom tests
in case the manufacturer doesn't exist
…ries_number_excluded
| allowed_manufacturers: | ||
| - regex: ".*" | ||
| exclude_series_numbers: [] |
There was a problem hiding this comment.
I think we should go through and find out all of the manufacturers before we allow the all through, as CT can have burned in data
| - regex: ".*" | ||
| exclude_series_numbers: [] | ||
|
|
||
| min_instances_per_series: 1 |
There was a problem hiding this comment.
Are we expecting any single instances? If not I'd suggest 2 so that screenshots aren't uploaded
| - regex: ".*" # allow all manufacturers | ||
| exclude_series_numbers: [] | ||
|
|
||
| min_instances_per_series: 1 |
There was a problem hiding this comment.
| min_instances_per_series: 1 | |
| min_instances_per_series: 2 |
| allowed_manufacturers: | ||
| - regex: ".*" # allow all manufacturers | ||
| exclude_series_numbers: [] |
There was a problem hiding this comment.
I think our template shouldn't allow all through, because that opens us up to exporting series which are burned in. But adding in canon's 8000 series to exclude is a good idea
Description
Closes #571
Closes #530
ensure all resources sharing aThis has already been fixed in Improve anon logging and anonymise all matching studies at once #582StudyInstanceUIDin Orthanc Raw are exported (they may exist as separate resources in Orthanc Raw if they have differentStudyIDs, but are combined into a single resource in Orthanc Anon as theStudyIDtag is deleted)Type of change
Please delete options accordingly to the description.
Suggested Checklist
mainbranch.squash and merge