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

Enhancements to Address #160

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

GhalebKhaled
Copy link

This PR tries to solve:

  • Google doesn't return locality every time even when address is pretty much defined, so we can rely on administrative_area_level_3 as fallback for city value. [Source from google] (https://developers.google.com/maps/documentation/places/web-service/supported_types#table3)
  • Not all cities in the world are located in state, such location can't fit in this library, e.g. Copenhagen, Denmark, proposed solution is to create dummy state to link city and country, i.e. locality -> unknown state-> country
  • Allowing dummy addresses can be problematic sometime, so I've added option to disallow them, set ALLOW_DUMMY_ADDRESSES to False in setting.py will stop saving dummy address, in my case, I wanted to raise error you use instead of silently storing dummy data
  • state_code with more than 8 chars is raising error, user has nothing to do other than selecting same address, so I've changed the handling to trim the extra chars, the state_code is invalid from google anyway so I just logged a warning.
  • modify pyproject.toml to fix pip install

@jplehmann
Copy link

@banagale this PR is our solution to Issue 15 -- #15 (comment) -- which we encountered when we migrated back from the dev branch recently to the main line.

Right now we are monkey patching the library but would prefer that everyone have these options. We have been using this in production for at least a month now without issues. We are using over 20k addresses in production.

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.

None yet

2 participants