-
Notifications
You must be signed in to change notification settings - Fork 15
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
Check for RR field presence before merging RR to eCR #866
Conversation
Codecov Report
@@ Coverage Diff @@
## main #866 +/- ##
==========================================
- Coverage 96.40% 96.37% -0.04%
==========================================
Files 46 46
Lines 2645 2648 +3
==========================================
+ Hits 2550 2552 +2
- Misses 95 96 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Everything looks good, but it may be better to search for the specific code ("2.16.840.1.113883.6.1"), rather than looking for text. But, either way, it should technically work.
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.
Awesome work!
This reverts commit 8a090df.
PULL REQUEST
Summary
Checks to see if RR data has already been extracted and inserted into the eCR before merging RR data into eCR. (Basically - have we already called this method?)
Related Issue
Fixes #844
Additional Information
Debated for a bit about raising a
ValueError
instead of just returning the eCR values, but this seemed cleaner - there's no need for the caller to know they called the method with a pre-merged eICR.