-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add fio2_percent and oxygen_flow_rate #188
Conversation
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.
thanks @pipliggins - lots of work!
all look very good; and just a few minor changes to look at ..
@sadiekelly would you mind checking the guinea parser particularly? It seems like it was merged before your review changes were applied, so it's worth going back and double checking I caught everything. |
isaric/parsers/ccp-drc.toml
Outdated
@@ -298,6 +298,11 @@ | |||
description = "Health worker?" | |||
ref = "Y/N/NK" | |||
|
|||
[subject.works_lab] | |||
field = "microb" | |||
description = "Employee of a microbiology laboratory" |
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.
@pipliggins I think for creating the works_lab supertype we'd only map the Y responses if the question specifically asks about working in a microbiology lab, as someone could respond 'N' to not working in a microbiology lab but they are still a lab worker, what do you think?
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.
@sadiekelly so you mean if a question asks 'are you a microbiology worker?' we'd only map the yes responses to works_lab, but both Y and N for works_microbiology_lab? Yes, I think that makes sense. We might potentially need to consider this logic for other super types as well...
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.
yes, that's right, I think that's the best way. Which of the other super types are you thinking of?
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 haven't thought it through... but possibly for things like oxygen therapy - @abhidg remind me, if a 'false' response is given within combinedType = any, will that override a 'True' response from earlier?
If the question asks specifically about a microbiology lab
@sadiekelly with the 2 new definitions here (oxygen_flow_rate and fio2) - do you think the presence of either/both should be used as an indicator of oxygen therapy in the visit table? Or just one? |
Summary of the missing optional fields for the ccp-cameroon parser:
|
Hi @sadiekelly, would you mind doing a final check through this? I think I've done everything we've discussed, but a second pair of eyes before I merge would be great! |
hi @pipliggins! I've added a few comments on the updated files - let me know if something is not clear - thanks! |
No description provided.