59 correct series invalidity #60
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
CavRiley
force-pushed
the
59-correct-series-invalidity
branch
from
1371194
to
0a340ea
Compare
mbrzus
reviewed
Sep 5, 2024
| @@ -627,6 +652,23 @@ def sanitize_dicom_dataset( | |||
| return dataset_dictionary, True | |||
|
|
|||
|
|
|||
| def validate_numerical_dataset_element(element: str) -> str | float: | |||
There was a problem hiding this comment.
ensure this function is able to sanitize None Type values as well
There was a problem hiding this comment.
additionally ansure unit testing for this function with different cases. Be careful using typehints and ensure that function won't crush the tool when encountering wrong type
mbrzus
reviewed
Sep 5, 2024
| dataset_dictionary[field] = dataset[field].value | ||
| dataset_dictionary[field] = validate_numerical_dataset_element( | ||
| dataset_value | ||
| ) | ||
| elif field == "Manufacturer": |
There was a problem hiding this comment.
for fields that do not use the new "validate_numerical_dataset_element" function, ensure that they are sanitized for the None Type
|
we have to ensure that the code is sanitized and test cover the None Type fields that I believe could be returned by pydicom in their DatasetElement object. |
ENH: added catch for returning None value from dataset elements
CavRiley
force-pushed
the
59-correct-series-invalidity
branch
from
727c66c
to
156d9e5
Compare
STYLE: black fixes TEST: testing verbose printing for dicom tag error ENH: separated operations for improved debugging FIX?: git lfs files are pulled from remote on github action
CavRiley
force-pushed
the
59-correct-series-invalidity
branch
from
156d9e5
to
c1974ee
Compare
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.
Overview
This pull request addresses a bug where invalid inputs to non-required fields are not being caught, resulting in an "INVALID" modality classification error.
Implementation
Testing
Problems Faced
There were issues deciding upon what was the best way to catch cases with optional input fields that were empty. We decided to initialize every optional value in the data set dictionary with a default empty value and overrode the value if provided.
Notes
N/A