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.
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
Refactor doctor_visits: Load source file only once #1978
base: main
Are you sure you want to change the base?
Refactor doctor_visits: Load source file only once #1978
Changes from 20 commits
c639049
1dd80be
749ed2d
9d8d521
ca38bb7
17259d0
6d841da
4ec46df
9740899
aacc545
dbde5c7
1394d3d
dfc3be2
e07c697
d1ee4ce
fc2c58d
b52d80a
58b51a6
81381d6
bfa853a
073651f
dd06a91
4ddd5a0
593279b
9920821
cd83691
79c34d3
7896042
e2f7953
a4f67c0
9408c81
b2f8b0e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
nit (optional): these filters (here and the two below) aren't actually changing the behavior since if there were any null or too high values, the assertion would cause the program to error out.
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.
todo: Please add brief rationale/source for this threshold of 5 in a comment. (The threshold of 90 below is self-evident and doesn't need a comment.)
If the threshold value came from the old code and was not explained, no need to do anything here.
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.
discussion: consider consistency with past behavior of this indicator and with behavior of other indicators. I know some of our other indicators reported any odd (negative, etc) values that came in from the source. I'm somewhat wary of just dropping these points.
Second, if the
df["val"] > 0
part of this filter is always desired behavior, it seems like it should also be done in theelse
block below. Right now theif not se
case doesn't do any filtering by value.Third, is
val == 0
not valid?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.
todo: Please expand/clarify this logging message. Our filter is doing more than removing
p == 0
andse == 0
points. This could perhaps include the currentgeo_id
as in aboveassert
messages and/or a count of affected rows.