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

Localize images from Markdown metadata #525

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

stasinos
Copy link

Read Lat, Lon fields in the Markdown files to localize images. These override the EXIF GPS coordinates, if both are present, allowing manually localizing the images on the map. This is useful to, for instance, localize the image based on what is depicted instead of where the camera was standing.

GPS coordinates can be given in Lat,Lon fields
in the Markdown files.
These override the EXIF GPS coordinates, if
both are present.
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.70%. Comparing base (c5c57fe) to head (664fd0a).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/sigal/gallery.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #525      +/-   ##
==========================================
- Coverage   90.72%   90.70%   -0.02%     
==========================================
  Files          24       24              
  Lines        2026     2034       +8     
==========================================
+ Hits         1838     1845       +7     
- Misses        188      189       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Owner

@saimn saimn left a comment

Choose a reason for hiding this comment

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

Seems useful, thanks. A few comments below.

docs/image_information.rst Outdated Show resolved Hide resolved
src/sigal/gallery.py Outdated Show resolved Hide resolved
src/sigal/gallery.py Outdated Show resolved Hide resolved
@@ -286,6 +287,15 @@ def _get_markdown_metadata(self):

return meta

@cached_property
def lat(self):
return self.markdown_metadata.get("lat", {})
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change the properties to return media.exif.gps.lat/lon if not available in meta ? this would avoid the if/else in the template.

Copy link
Author

Choose a reason for hiding this comment

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

But that would make it impossible to write a template that prefers the EXIF coordinates.
You might not want this, IMHO.

Copy link
Owner

Choose a reason for hiding this comment

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

people can still use directly media.exif.gps. But for media.lat I think it makes sense to avoid the manual coordinates or the exif ones.

Copy link
Author

Choose a reason for hiding this comment

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

I hate to insist, but I feel I have to. I perceive the Python code as a considerably "deeper" layer than the templates. I expect that many users will be happy to work at the template layer but will want to avoid the Python layer. Thus, I expect that a simple, easily predictable behaviour for media.lat/lon and media.exif.gps makes it easy to write templates.

Now, if you really want to have a template without ifs, I suggest keeping meta.lat/lon as they are and adding a new set of variables that implement the fallback and a configuration option that defines preference to GPS or .md if both are present.

Copy link
Author

Choose a reason for hiding this comment

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

And, BTW, come to think of it, one of the ifs will still be needed as it might be the case that there are no coordinates at either EXIF or .md. So we will still need an if, except without the else branch.

Copy link
Author

Choose a reason for hiding this comment

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

I just had another idea: How about a config option that sets the order of preference between md, EXIF?
Then both ways are accessible at the template. If the option also includes a "default location" (say, 0,0 or someplace in the middle of the Pacific) then all photos are guaranteed to have a location and the template can be completely without checks. This has the advantage that photos without a location do not silently disappear but are placed at a location that the users know to mean that they have forgotten to add a location.

stasinos and others added 2 commits November 19, 2024 18:30
Slight code simplification, allowing get() to
return None when the field is missing. The related
if statement in Jinja2 accepts None as False.

Co-authored-by: Simon Conseil <[email protected]>
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