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

fix(server): Improve reverse geocoded location metadata #9051

Merged
merged 4 commits into from
Apr 28, 2024

Conversation

hermesespinola
Copy link
Contributor

@hermesespinola hermesespinola commented Apr 24, 2024

Description

  • Limit state field to admin1Name (1st level country divisions).
  • Add filter to remove 'PPLX' Feature codes from cities, which is classified as "section of populated place" (see https://www.geonames.org/export/codes.html).
  • Updated tests to reflect these changes.
  • Improves the reverse geocoding features. There are still edge cases.

Fixes #8941

How Has This Been Tested?

  • Download photos from my Immich server.
  • Upload theses photos to the dev version.
  • Compare the difference between the two.

Screenshots (if appropriate):

After:

image
image
image

Before:

image
image
image

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable

@hermesespinola hermesespinola marked this pull request as ready for review April 28, 2024 05:27
@bo0tzz bo0tzz requested a review from zackpollard April 28, 2024 08:12
@alextran1502
Copy link
Contributor

@zackpollard Feel free to merge this one after you have taken a look

@zackpollard
Copy link
Contributor

I like this change for the immediate effect, however in future would it be more beneficial to keep these in but process them differently, so have more fields for storing this geo data at a lower level than city?

@hermesespinola
Copy link
Contributor Author

What would be the use case for this data? I could create a migration to add this field to the geodata entities and filter them during reverse geocoding.

One of the main problems is that sometimes a pplx entity is close to the media location (i.e.: the San Francisco photo) even if that photo is not taken in that place (I guess because the reverse geocoding is based on proximity rather than geofencing).

@zackpollard
Copy link
Contributor

What would be the use case for this data? I could create a migration to add this field to the geodata entities and filter them during reverse geocoding.

One of the main problems is that sometimes a pplx entity is close to the media location (i.e.: the San Francisco photo) even if that photo is not taken in that place (I guess because the reverse geocoding is based on proximity rather than geofencing).

Yea, perhaps the real problem is that we don't have enough of that type of data. I say we merge this and revisit it once we look at importing the entire allCountries dataset which includes way more of this finer detail data 😄

@zackpollard zackpollard merged commit 4bb7d2d into immich-app:main Apr 28, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reverse geocoding is frequently incorrect
5 participants