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

Use 3-character country code for dependencies where possible #311

Merged
merged 1 commit into from
May 12, 2022

Conversation

orangejulius
Copy link
Member

After pelias/api#1622, which extends the boundary.country API parameter to include the dependency placetype, we noticed that venue and address records often have a 2-character country code when they are part of a dependency.

The boundary.country parameter currently checks only for 3-character codes, and so the best solution is to ensure the relevant property on all records in Elasticsearch is a 3-character country code whenever possible.

This change expands on the logic for selecting an abbreviation for an admin record to be used for point in polygon lookup. Logic specific to countries is expanded to include dependencies, and some additional possible fields that might contain 3-character codes are checked.

Because the abbreviation logic is now a bit more substantial, it's extracted into its own function. It's also been made a bit less redundant and hopefully more clear.

…ere possible

After pelias/api#1622, which extends the
`boundary.country` API parameter to include the `depencency` placetype,
we noticed that venue and address records often have a 2-character
country code when they are part of a dependency.

The `boundary.country` parameter currently checks only for 3-character
codes, and so the best solution is to ensure the relevant property on
all records in Elasticsearch is a 3-character country code whenever
possible.

This change expands on the logic for selecting an abbreviation for an
admin record to be used for point in polygon lookup. Logic specific to
countries is expanded to include dependencies, and some additional
possible fields that might contain 3-character codes are checked.

Because the abbreviaton logic is now a bit more substantial, it's
extracted into its own function. It's also been made a bit less
redundant and hopefully more clear.
Copy link
Member

@missinglink missinglink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@orangejulius orangejulius changed the title feat(abbreviations): Use 3-character country code for dependencies where possible Use 3-character country code for dependencies where possible May 12, 2022
@orangejulius orangejulius merged commit c194aec into master May 12, 2022
@orangejulius orangejulius deleted the dependency-country-code branch May 12, 2022 20:13
orangejulius added a commit to pelias/csv-importer that referenced this pull request May 12, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
orangejulius added a commit to pelias/openstreetmap that referenced this pull request May 16, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
orangejulius added a commit to pelias/openaddresses that referenced this pull request May 16, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
orangejulius added a commit to pelias/polylines that referenced this pull request May 16, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
orangejulius added a commit to pelias/transit that referenced this pull request May 16, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
orangejulius added a commit to pelias/pip-service that referenced this pull request May 16, 2022
This includes the changes in
pelias/wof-admin-lookup#311 that help with using
the `boundary.country` API parameter with `dependency` placetypes.
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