-
Notifications
You must be signed in to change notification settings - Fork 116
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
Drop counselors with invalid zips when geocoding #8718
Conversation
15a1239
to
399b17f
Compare
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 for the fix.
cfgov/housing_counselor/geocoder.py
Outdated
logger.warning(f"{zipcode} not in zipcodes") | ||
return False | ||
|
||
return True | ||
|
||
def geocode_counselor(self, counselor): | ||
counselor = dict(counselor) |
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.
Can this line be removed since you already ensure the counselor
object is a dict in filter_zipcodes
?
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 don't think so, since the filter just removes entries from the list of counselors but doesn't modify them. I could do this to the whole list ahead of time though
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.
Did the above in the following two commits
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.
The only thing I wonder about is how information about bad data could make its way back to the source. Seems to be relatively rare, so maybe not a concern.
Currently, if when the housing_counselor mgmt command attempts to geocode data with an invalid zipcode, it throws a
KeyError
. This bails out of the command and thus causes our daily update to fail.Since we don't control this data, I think it would be better to simply drop the counselor with the invalid zip (while logging that this is being done). This should allow us to be more resilient to erroneous data creeping into the HUD list of counselors.