Skip to content

[FIX] consistently refer to DICOM Tags throughout the specification#786

Merged
sappelhoff merged 12 commits intobids-standard:masterfrom
Hboni:master
May 20, 2021
Merged

[FIX] consistently refer to DICOM Tags throughout the specification#786
sappelhoff merged 12 commits intobids-standard:masterfrom
Hboni:master

Conversation

@Hboni
Copy link
Copy Markdown
Contributor

@Hboni Hboni commented Apr 23, 2021

Only some changes for Dicom tag representation to be consistent across the page and as used in MRI.
Also, remove what I think is duplicate in a figure caption

tsalo added a commit that referenced this pull request Apr 26, 2021
@Hboni Hboni requested a review from chrisgorgo as a code owner May 14, 2021 09:39
@sappelhoff sappelhoff changed the title [FIX] typos in PET Dicom tags [FIX] consistently refer to DICOM Tags throughout the specification May 14, 2021
Copy link
Copy Markdown
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks for seeing this and fixing a bunch of these inconsistencies @Hboni.

I took your start and made a pass through the whole spec, so that we always use DICOM Tag \d+, \d+

Also thanks for removing that duplication in the caption!

PS: You made this PR from your master branch. Just for next time, I recommend that you make a separate branch off your (up to date) master branch to submit changes/proposals --> that can avoid some git difficulties that we luckily didn't run into here (so far)

@sappelhoff sappelhoff requested a review from tsalo May 14, 2021 09:45
@sappelhoff sappelhoff added the formatting Aesthetics and formatting of the spec label May 14, 2021
@sappelhoff sappelhoff requested a review from effigies May 19, 2021 12:42
@satra
Copy link
Copy Markdown
Collaborator

satra commented May 19, 2021

so that we always use DICOM Tag \d+, \d+

@sappelhoff - while it doesn't matter from a doc perspective, just a note that dicom tags are hexadecimal so [a-e\d]{4} or 0x[a-e\d]{4} if you want to make it explicit (in a programming sense).

sappelhoff and others added 2 commits May 19, 2021 17:17
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@sappelhoff
Copy link
Copy Markdown
Member

Thanks both. To be clear: I don't know how we would want DICOM tags to be formatted in the spec. I am simply completing/fixing up @Hboni's start to make the formatting consistent based on which convention was initially used in the spec. Prior to PET, ASL, qMRI adding some more tags.

@sappelhoff sappelhoff merged commit f7fd3c2 into bids-standard:master May 20, 2021
tsalo added a commit that referenced this pull request Jul 13, 2021
* [SCHEMA] Add metadata term files (#762)

* Draft a handful of metadata term files.

* Add example with specific possible values.

* Match the validator schemas better.

* Fix formatting.

* Fix formatting again!

* Draft semi-functional rendering functions.

* Use unit abbreviations.

* Add more fields.

* Get macro working.

* Add terms from first table.

* Add AnatomicalLandmarkCoordinateSystem.

* fMRI task information table.

* More terms.

* More terms.

* More terms.

* EchoTime and FlipAngle

* Add tables.

* More tables.

* More terms.

* More terms.

* More terms.

* Fix spacing.

* Clean things up.

* More terms.

* More terms!

* More terms.

* Some iEEG terms.

* More iEEG terms.

* Add ASL labeling terms.

* Next batch.

* More terms.

* More terms.

* More terms.

* Fix mistakes.

* Last terms.

* Reference yamls.

* Fix mistakes.

* Change format of coordinate system files.

* Use degree in associated files.

* Some of the missing terms.

* A few more terms.

* Fix typos.

* More terms.

* More terms.

* More terms.

* More terms.

* More terms.

* More terms.

* Last terms.

* Fix link.

* Fix internal links.

* Fix links for real.

* Derivative terms.

* Fix up code link.

* Use backslashes for continued strings.

* Replace $ref with file contents.

Also support plural datatype strings and all manner of newlines in descriptions.

* Fix genetics.

* Describe the structure of metadata YAML files.

* Make metadatatype function recursive.

* Improve search function.

* Start adding PET fields.

* Add some fields.

* More terms.

* More terms.

* More terms.

* Fix mistakes.

* More terms.

* Replace InstitutionDepartmentName with existing InstitutionalDepartmentName.

* More terms.

* More terms.

* More terms.

* More terms.

* More terms.

* More terms.

* More terms.

* Last terms.

* Add unit format for strings.

Unused for now but could be useful later.

* Add dataset_relative and participant_relative string formats.

* Update READMEs.

* Fix formats in README.

* Support table-specific metadata description extensions.

* Employ description extensions with IntendedFor.

* Remove explicit defaults from YAML files.

* Replace Minimum with minimum.

* Replace inclusiveMaximum with maximum.

* Replace implicit links with explicit ones.

* Rename key_name to name.

* Rename "Unit" to "Units"

* Improve make_metadata_table docstring.

* Start addressing inconsistencies between rendered and hardcoded tables.

* Fix typos in PET metadata

From #786.

* Add metadata fields from qMRI appendix.

* Fix.

* Address duplicate datatypes.

Should address the "string or string or string or string" issue.

* Wrap example strings in code.

* Use enum for n/a instead of pattern.

It's easier to identify as a special case.

* Replace "string" with "n/a" when appropriate.

* Address some inconsistencies.

* Take a crack as SpatialReference.

The type is complicated, but I _think_ I've got it figured out.

* Apply suggestions from code review

Thanks @effigies!

Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>

* Update tools/schemacode/schema.py

* search_structure --> dereference_yaml

* Use faster loading approach.

* Fix deprecation link.

* Apply suggestions from code review

Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>

* Update 01-magnetic-resonance-imaging-data.md

* Add B0FieldIdentifier and B0FieldSource.

* Revert type changes and add TODOs to check them.

* Update tools/schemacode/schema.py

* Replace remaining relative links.

* Apply suggestions from code review

Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>

* Add new coordinate systems from #775.

* Grab hack from #781 (which wasn't merged).

* Create HED.yaml

* boldify table headers

* add device info metadata

* fix table fences

* fix cell padding

* fix cell padding

this is getting old VERY quickly

* Fix up DICOM tags in metadata.

* Leverage "name" field for section-specific metadata definitions.

* composite instances --> measurements

Changes from #813.

* Fix name of HED field.

* Fix string formatting in coordinate system fields.

* Move "preferably same as" to section-specific text.

For #774 (comment)

* Standardize DICOM Tag format.

Still need to move the references out of the generic definitions.

* Move mentions of DICOM Tags out of definitions.

Only for fields that *also* appear in modalities that *don't* use DICOM.

* Apply suggestions from code review

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Generalize SoftwareFilters example.

* Distinguish AnatomicalLandmarkCoordinates definitions.

* Rename fmapEchoTime to match new format.

* Apply suggestions from code review

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Apply suggestions from code review

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Address review.

* Fix example manufacturer names.

* TEMPORARY: fix osipi URL (revert when osipi.org is back)

* Fix regex for identifying macros.

* Partially address review.

* Do not assume minItems is 1 for array terms.

* composite instances --> measurements (again)

* Update src/schema/metadata/MagneticFieldStrength.yaml

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Update description to PharmaceuticalDoseTime

#774 (comment)

* Update 09-positron-emission-tomography.md

* fix table pipe alignment

* add pharmaceuticaldosetime fix to schema

includes a bugfix to convert "should" (not casing, this was not intended
as a SHOULD) to a MUST.

* fix str examples

* Apply suggestions from code review

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Update src/schema/metadata/RepetitionTimeExcitation.yaml

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Add DoseCalibrationFactor.

* Update ScanDate definition and deprecate it.

* Remove hardcoded tables.

* Remove unused links.

* Update tools/schemacode/schema.py

Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>

Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>
Co-authored-by: Remi Gau <remi_gau@hotmail.com>
Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
Co-authored-by: mnoergaard <martin.noergaard@nru.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

formatting Aesthetics and formatting of the spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants