Skip to content
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 and add CSVs for additional zips #36

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

philvarner
Copy link
Contributor

@philvarner philvarner commented Feb 2, 2021

  1. Primary fix is for Some zipcodes with PHZ data are not in zipcodes.csv file, and therefore have no api json file #35 to add a CSV (combined_zipcodes.csv) that merges a larger ZIP csv (us-zip-code-latitude-and-longitude.csv) with the existing one. This covers many more zips that are in the PHZ data but didn't have location info before.
  2. The refactor of the code is pretty significant, and wholly unnecessary, but was primarily just a motivating case to experiment in my own Python learning.

@waldoj
Copy link
Owner

waldoj commented Feb 2, 2021

Thank you, Phil! I'm going to get this to build (I seem to have some configuration things wrong) before I merge it. I only did a 5-minute review, but this all seems like a big improvement. :)

@waldoj waldoj merged commit 5bc72e2 into waldoj:master Feb 3, 2021
@waldoj
Copy link
Owner

waldoj commented Feb 3, 2021

...or merge and then build because life is short and the risk is low with an API made out of static files. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants