Use state abbreviations in notification

alex / nyt-2020-election-scraper

https://alex.github.io/nyt-2020-election-scraper/battleground-state-changes.html

MIT License

1.76k stars 289 forks source link

Use state abbreviations in notification #379

Closed berkowitze closed 3 years ago

berkowitze commented 3 years ago

Motivation

Closes #378

Changes

Before: After:

[x] Ensured that you have rebased your branch with this repo's latest master branch.
[x] Ensured that relevant issues are linked, if this PR resolves any outstanding.
[x] Added a screenshot for all UI changes (you can drag the file into this edit box and it will be uploaded).
[x] Ensured that changes to auto-generated files have not been committed; in particular, *.html, *.csv, *.xml and *.json files.

sebsebmc commented 3 years ago

Are American Samoa and Federated States of Micronesia needed here? I don't believe they are in the data we use

Same with Guam, Marshall Islands, etc.

berkowitze commented 3 years ago

True, got this object from stackoverflow. Figured extra states couldn't hurt 🤷‍♂️

Happy to remove them.

sebsebmc commented 3 years ago

DC exists in the 50 state code, so its actually useful, but I believe that the state abbreviation are in the JSON data already so we don't need this translation object at all.

berkowitze commented 3 years ago

@sebsebmc the abbreviations are kind of buried in results.json - I feel like it's not worth parsing all of that just to get the abbreviation. Unless the state names are inconsistent over time/don't exactly match the translation object.

saleemrashid commented 3 years ago

Thanks for the great pull request! Since results.json includes the state abbreviations that we need, I'd prefer to use that instead of hardcoding the abbreviations in the codebase.

I'd like to close this in favor of #380, which hopefully fixes your issue.

titanous commented 3 years ago

Superceded by #380

berkowitze commented 3 years ago

Makes sense, cheers!