-
Notifications
You must be signed in to change notification settings - Fork 170
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
[ENH] Add metadata fields for DeIdentificationMethod/CodeSequence for MRI and PET #1772
[ENH] Add metadata fields for DeIdentificationMethod/CodeSequence for MRI and PET #1772
Conversation
…uence, corresponding to the same-named DICOM tags. These record info on de-identification, including metadata and image content (e.g. de-facing or removing burned-in text)
This addresses issue #1709 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1772 +/- ##
=======================================
Coverage 88.22% 88.22%
=======================================
Files 16 16
Lines 1393 1393
=======================================
Hits 1229 1229
Misses 164 164 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, to validate these, we will need to add them to sidecars.
I would suggest a pair of new tables, such as:
DeidentificationMethod:
selectors:
- intersects([modality], ["mri", "pet"])
fields:
DeidentificationMethod: optional
DeidentificationMethodCodeSequence:
level: optional, but recommended if DeidentificationMethod is present
DeidentificationMethodRec:
selectors:
- intersects([modality], ["mri", "pet"])
- sidecar.DeidentificationMethod
fields:
DeidentificationMethodCodeSequence: recommended
Then for rendering, these can then be added to the lists of tables for MRI data:
bids-specification/src/modality-specific-files/magnetic-resonance-imaging-data.md
Lines 16 to 176 in d7b3793
### Hardware information | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRIHardware") }} | |
Example for `ReceiveCoilActiveElements`: | |
For Siemens, coil channels are typically not activated/selected individually, | |
but rather in pre-defined selectable "groups" of individual channels, | |
and the list of the "groups" of elements that are active/selected in any | |
given scan populates the `Coil String` entry in Siemens' private DICOM fields | |
(for example, `HEA;HEP` for the Siemens standard 32 ch coil | |
when both the anterior and posterior groups are activated). | |
This is a flexible field that can be used as most appropriate for a given | |
vendor and coil to define the "active" coil elements. | |
Since individual scans can sometimes not have the intended coil elements selected, | |
it is preferable for this field to be populated directly from the DICOM | |
for each individual scan, so that it can be used as a mechanism for checking | |
that a given scan was collected with the intended coil elements selected. | |
### Institution information | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRIInstitutionInformation") }} | |
### Sequence Specifics | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRISequenceSpecifics") }} | |
### In- and Out-of-Plane Spatial Encoding | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table(["mri.MRISpatialEncoding", "mri.PhaseEncodingDirectionRec"]) }} | |
<sup>2</sup>Conveniently, for Siemens data, this value is easily obtained as | |
`1 / (BWPPPE * ReconMatrixPE)`, where BWPPPE is the | |
"BandwidthPerPixelPhaseEncode" in DICOM Tag 0019, 1028 and ReconMatrixPE is | |
the size of the actual reconstructed data in the phase direction (which is NOT | |
reflected in a single DICOM Tag for all possible aforementioned scan | |
manipulations). See | |
[Acquiring and using field maps - LCNI](https://lcni.uoregon.edu/wiki/acquiring-and-using-field-maps/) | |
and [TotalReadoutTime - dcm\_qa](https://github.com/neurolabusc/dcm_qa/tree/master/In/TotalReadoutTime). | |
<sup>3</sup>We use the time between the center of the first "effective" echo | |
and the center of the last "effective" echo, sometimes called the "FSL definition". | |
### Timing Parameters | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table(["mri.MRITimingParameters", "mri.SliceTimingMRI"]) }} | |
### RF & Contrast | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table(["mri.MRIFlipAngleLookLockerFalse", "mri.MRIRFandContrast" ]) }} | |
### Slice Acceleration | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRISliceAcceleration") }} | |
### Anatomical landmarks | |
Useful for multimodal co-registration with MEG, (S)EEG, TMS, and so on. | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRIAnatomicalLandmarks") }} | |
### Echo-Planar Imaging and *B<sub>0</sub>* mapping | |
Echo-Planar Imaging (EPI) schemes typically used in the acquisition of | |
diffusion and functional MRI may also be *intended for* estimating the | |
*B<sub>0</sub>* field nonuniformity inside the scanner (in other words, | |
*mapping the field*) without the acquisition of additional MRI schemes | |
such as gradient-recalled echo (GRE) sequences that are stored under the | |
`fmap/` directory of the BIDS structure. | |
The modality labels `dwi` (under `dwi/`), `bold` (under `func/`), | |
`asl` (under `perf/`), `sbref` (under `dwi/`, `func/` or `perf/`), and | |
any modality under `fmap/` are allowed to encode the MR protocol intent for | |
fieldmap estimation using the following metadata: | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRIEchoPlanarImagingAndB0Mapping") }} | |
#### Tissue description | |
<!-- This block generates a metadata table. | |
These tables are defined in | |
src/schema/rules/sidecars | |
The definitions of the fields specified in these tables may be found in | |
src/schema/objects/metadata.yaml | |
A guide for using macros can be found at | |
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md | |
--> | |
{{ MACROS___make_sidecar_table("mri.MRISample") }} | |
e.g.
### Deidentification information
<!-- This block generates a metadata table.
These tables are defined in
src/schema/rules/sidecars
The definitions of the fields specified in these tables may be found in
src/schema/objects/metadata.yaml
A guide for using macros can be found at
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md
-->
{{ MACROS___make_sidecar_table("mri.DeidentificationMethod") }}
Each object in the `DeidentificationMethodCodeSequence` array includes the following RECOMMENDED keys:
<!-- This block generates a table describing subfields within a metadata field.
The definitions of these fields can be found in
src/schema/objects/metadata.yaml
and a guide for using macros can be found at
https://github.com/bids-standard/bids-specification/blob/master/macros_doc.md
-->
{{ MACROS___make_subobject_table("metadata.DeidentificationMethodCodeSequence.items") }}
And we should do the same for PET.
I apologize for missing some pieces, and I think I understand, now. Thank you for your careful review and for helping me through this process. I will execute your suggestions and amend the PR with another commit. |
No need to apologize! I appreciate the effort you're putting into this. |
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
for more information, see https://pre-commit.ci
Chris, I committed your suggested diffs as-is. For the tables and sidecars, I deviated a bit in calling both tags optional. My read of DICOM is that if PatientIdentityRemoved is TRUE (which is a tag we don't have in BIDS, and I don't think BIDS really needs it), then at least one of either DeidentificationMethod or DeidentificationCodeSequence is required. Without PatientIdentityRemoved, I would say neither is required, and both are optional. It's not that DeidentificationMethod is recommended if DeidentificationCodeSequence is present, but rather they are of equal status and people can use either. DeidentificationMethod is a single simple string, but there's a greater chance that some downstream software would replace rather than amend the string. DeidentificationCodeSequence better accounts for there being a series of de-identification steps and other software would more likely amend than overwrite it, but it's also a sequence tag and those are unpleasantly messy. While I was copying from the MRI to the PET table, I noticed the "defacemask" block in the MRI but not PET. I copied this to the PET, since de-facing is also routinely applied to PET now. I looked for CT-related files, but it appears that CT is not a part of BIDS currently? If CT is ever added, my recommendation is that these elements all be added to CT to match MRI and PET. Thanks for your help and careful review! |
Sounds good.
Could you split that out into a separate PR? I think it will need some additional work to apply properly to PET, and I'd rather not complicate this PR more than necessary.
Correct, and agreed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small suggestions. Also, in case you haven't seen it hiding in the review boxes (see image), here's the rendered specification, which will update a few minutes after you make pushes to this PR.
Co-authored-by: Chris Markiewicz <[email protected]>
for more information, see https://pre-commit.ci
Chris, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this from a schema and rendering perspective, and I believe this should be relatively simple to implement for DICOM converters.
I would appreciate if at least a couple representatives of converters could weigh in:
(I know you're all busy, please feel free to delegate.)
For those who would rather read spec than schema:
Thanks Chris (Markiewicz)! I have a companion PR for dcm2niix at rordenlab/dcm2niix#813, but Chris (Rorden) hasn't commented on it yet. I had initially asked him to add the tags to dcm2niix's .json, and he said to get them added to bids-specification first, so that's how I arrived here. I don't have any involvement with the other converters. Thanks, |
type: string | ||
description: | | ||
An identifier of the version of the coding scheme if necessary to resolve ambiguity. | ||
Corresponds to DICOM Tag 0008, 0103 `Coding Scheme Version`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is complicated enough warranting addition to https://github.com/bids-standard/bids-examples if not as an example here in the BIDS spec text. Do we have some nice example DICOMs to check those fields out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if someone were creating these tags from scratch at the BIDS level, they would use DeidentificationMethod rather than DeidentificationMethodCodeSequence. The latter is more provided to allow direct translation by dcm->nii and dcm->bids software.
I'm attaching some nice example DICOM, as requested. This is MRI of a spaghetti squash (thanks @captainnova), and I've further de-identified it, so there's no PHI. I included the tags as if I had de-faced it, but it is a squash, so there was no face to de-face.
The relevant dcm tags look like this:
(0012, 0063) De-identification Method LO: ['mri_reface 0.3.4', 'Per DICOM PS 3.15 AnnexE. Details in 0012,0064']
(0012, 0064) De-identification Method Code Sequence 5 item(s) ----
(0008, 0100) Code Value SH: '113102'
(0008, 0102) Coding Scheme Designator SH: 'DCM'
(0008, 0103) Coding Scheme Version SH: '01'
(0008, 0104) Code Meaning LO: 'Clean Recognizable Visual Features Option'
---------
(0008, 0100) Code Value SH: 'replace_recognizable'
(0008, 0102) Coding Scheme Designator SH: 'mri_reface'
(0008, 0103) Coding Scheme Version SH: '0.3.4'
(0008, 0104) Code Meaning LO: 'Replace face, ears, and artifacts in air'
---------
(0008, 0100) Code Value SH: '113100'
(0008, 0102) Coding Scheme Designator SH: 'DCM'
(0008, 0104) Code Meaning LO: 'Basic Application Confidentiality Profile'
---------
(0008, 0100) Code Value SH: '113107'
(0008, 0102) Coding Scheme Designator SH: 'DCM'
(0008, 0104) Code Meaning LO: 'Retain Longitudinal Temporal Information Modified Dates Option'
---------
(0008, 0100) Code Value SH: '113111'
(0008, 0102) Coding Scheme Designator SH: 'DCM'
(0008, 0104) Code Meaning LO: 'Retain Safe Private Option'
---------
With rordenlab/dcm2niix#813, the relevant portion of json output is:
"DeidentificationMethod": "mri_reface 0.3.4\\Per DICOM PS 3.15 AnnexE. Details in 0012,0064",
"DeidentificationMethodCodeSequence": [
{
"CodeValue": "113102",
"CodingSchemeDesignator": "DCM",
"CodingSchemeVersion": "01",
"CodeMeaning": "Clean Recognizable Visual Features Option"
},
{
"CodeValue": "replace_recognizable",
"CodingSchemeDesignator": "mri_reface",
"CodingSchemeVersion": "0.3.4",
"CodeMeaning": "Replace face, ears, and artifacts in air"
},
{
"CodeValue": "113100",
"CodingSchemeDesignator": "DCM",
"CodeMeaning": "Basic Application Confidentiality Profile"
},
{
"CodeValue": "113107",
"CodingSchemeDesignator": "DCM",
"CodeMeaning": "Retain Longitudinal Temporal Information Modified Dates Option"
},
{
"CodeValue": "113111",
"CodingSchemeDesignator": "DCM",
"CodeMeaning": "Retain Safe Private Option"
}
],
I just want to check in on the status here. It looks like there aren't objections, but are we waiting on a PR to bids-examples, or an agreement in principle RE rordenlab/dcm2niix#813? |
src/schema/objects/metadata.yaml
Outdated
description: | | ||
A description of the mechanism or method used to remove the Patient's identity. | ||
Corresponds to DICOM Tag 0012, 0063 `De-identification Method`. | ||
type: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following up on rordenlab/dcm2niix#813 (comment).
I'm not sure if this practically shows up in DICOM as a list of strings, or if it is either a string or a list of strings.
If we want to generally make this an array of strings, it would be:
type: string | |
type: array | |
items: | |
type: string |
If it would be a string or an array of strings:
type: string | |
anyOf: | |
- type: string | |
- type: array | |
items: | |
type: string |
I tend to prefer the simpler array of strings, rather than allowing it to sometimes be a string, but I'm okay following what DICOM does, for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be happy to approve this and get it merged once the type is updated per @effigies suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am at a conference this week. I can make the change next week when I return.
Thank you both for your review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed DeidentificationMethod to an array of strings, as requested. In DICOM, it is an array of strings where the size may be 1 to n, so I think an array of strings better matches DICOM than "might be a string and might be an array of strings". Please let me know if anything else is needed. Thanks!
This looks great as it would greatly speed up my ability to determine if PET images have been defaced or not. Will very happily add this to PET deface as soon as it's merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thank you for submitting this!
Add tags for DeIdentificationMethod and DeIdentificationMethodCodeSeuence, corresponding to the same-named DICOM tags. These record info on de-identification, including metadata and image content (e.g. de-facing or removing burned-in text)