Alternative SCImage Constructor #97
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hackermd
reviewed
Aug 9, 2021
src/highdicom/sc/sop.py
Outdated
| @@ -367,3 +367,158 @@ def __init__( | |||
| self.PixelData = encapsulate([encoded_frame]) | |||
| else: | |||
| self.PixelData = encoded_frame | |||
|
|
|||
| @classmethod | |||
| def from_study_dataset( | |||
There was a problem hiding this comment.
I find the name study_dataset confusing, since a data set encodes an instance. Can we call it reference_dataset or ref_dataset instead?
There was a problem hiding this comment.
Totally agree. I knew study_dataset wasn't great but was drawing a blank on something better and was hoping you could suggest one. Thanks! Changed the method and its parameter to ref_dataset as suggested in 5cce71d.
hackermd
approved these changes
Aug 10, 2021
hackermd
added a commit
that referenced
this pull request
Aug 31, 2021
* Integrate new pydicom DS formatting * Fix recording of evidence in structured reports * Fix mypy errors * Add test for report referencing multiple studies * Add properties to content items * Use datetime workaround for python 3.6 support * Add properties and methods on SR templates * Update type of content item property return value * Fix typo in exception message * Fix recording of evidence in structured reports * Update package version * Add image library entries to SR documents (#77) * Fixed enum RelationshipType * Fix handling of library entry descriptors * Fix default value for Pixel Origin Interpretation * Implement indexing by dimension index values * Rename tracking id methods * Add check for correct segment numbers * Fix implementation of content property on SR documents * Make value and unit required for NUM content items * Fix return value type of properties of NUM content item * Add qualifier property to NUM content item * Return uids of type UID instead of str * Simplify reshaping of graphic data array * Add is_root parameter to alternative constructor * Allow constructor of UID to accept a value * Chang Optional[x] -> Union[x, None] in docstrings * Log warning if wrong number of content items * Add property for temporal range type * Move QualitativeEvaluation from content to templates * Allow Concept Name Code Sequence to be absent * Require Concept Name Code Sequence for CONTAINER * Let pydicom handle encapsulated frame decoding * Write error traceback to stderr instead of stdout * Add docstrings for enums * Implementation of Parametric Map IOD (#89) * Changed PatientSexValues to single letters * Expose enum at package root * Remove sphinx duplication for highdicom.enum * Alternative SCImage Constructor (#97) * Only warn if patient name check fails Co-authored-by: hackermd <[email protected]> Co-authored-by: Christopher Bridge <[email protected]> Co-authored-by: Sean Doyle <[email protected]> Co-authored-by: Sean Doyle <[email protected]> Co-authored-by: Markus D. Herrmann <[email protected]> Co-authored-by: Chris Gorman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current constructor for the SCImage requires several pieces of patient or study related metadata. Often in practice, this information is simply copied from another dataset in the study in user code, but this process is annoying and verbose. I have now done this in several places personally.
This PR introduces a new alternative constructor for SCImage called
from_study_dataset(open to other suggestions for the name), that accepts another dataset from the study and copies the patient and study related metadata from there. This should make user code more concise and less error prone for this very common situation. Also added tests.