Skip to content

Conversation

@patrickfournier
Copy link
Collaborator

@patrickfournier patrickfournier commented Nov 12, 2025

This pull request adds support for processing image paths found in metadata fields (such as og_image) in addition to images in HTML content. It introduces a new configuration option to specify which metadata fields should be processed and which transformations to apply. The implementation includes the new processing logic, updates to the documentation, and corresponding tests.

New feature: Process images in metadata fields

  • Added the ability to process images referenced in metadata fields by introducing the IMAGE_PROCESS_METADATA setting. This allows users to specify which metadata fields should be processed and which image transformation to apply. The processing supports per-instance overrides and saves original metadata values for reference. (README.md, pelican/plugins/image_process/image_process.py) [1] [2]

Documentation updates

  • Updated the documentation to explain how to use the new IMAGE_PROCESS_METADATA setting, including examples and override mechanisms. (README.md) [1] [2]

Testing

  • Added parameterized tests for the new metadata processing logic, covering default behavior, transformation application, and per-instance overrides. (pelican/plugins/image_process/test_image_process.py)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces metadata image processing functionality to the Image Process plugin, allowing users to process images referenced in metadata fields (such as og_image) in addition to images in HTML content. The implementation adds a new configuration setting, updates documentation, and includes comprehensive tests.

  • Adds IMAGE_PROCESS_METADATA setting to specify which metadata fields should be processed and which transformations to apply
  • Supports per-instance transformation overrides using {transform-name} prefix syntax
  • Refactors compute_paths function to accept image URL strings instead of dictionary objects

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
pelican/plugins/image_process/image_process.py Implements metadata processing logic with the new process_metadata function, refactors compute_paths signature to accept strings, and registers the new signal handler
pelican/plugins/image_process/test_image_process.py Adds parameterized tests for metadata processing covering default behavior, transformation application, and override mechanisms; updates tests to use refactored compute_paths signature
README.md Documents the new metadata processing feature with usage examples and configuration details

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

In Python 3.14, `urllib.parse.urljoin` handles `//` differently than in previous versions.
@patrickfournier patrickfournier marked this pull request as ready for review November 13, 2025 05:20
Copy link
Contributor

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

I made a small suggestion for the README, but otherwise this looks good to me. Any thoughts, @minchinweb?

Copy link
Member

@minchinweb minchinweb left a comment

Choose a reason for hiding this comment

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

A couple thoughts on how to make the documentation a little clearer. The plugin is hugely powerful, but annoying complex to get set up from scratch, so the easier we can make that, they better.

And a request to guard against reusing "special" "keys", already is use elsewhere.

IMAGE_PROCESS_METADATA = {
"og_image": "og-image-transform",
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide an example IMAGE_PROCESS here, showing the listed transformation (maybe scale to the proper dimensions??)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

README.md Outdated
It is possible to override the transformation applied to a specific instance of a metadata field by prefixing
the metadata value with `{transform-name}`. For example, if you have defined
`IMAGE_PROCESS_METADATA` as above, you can override the transformation for a specific article
by setting its `og_image` metadata value to `{other-transform}/path/to/image.jpg`.
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a (simple) sample file (i.e. a Markdown post) showing this in the metadata of the post in the actual format?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

metadata field names to transformation names. For example:

```python
IMAGE_PROCESS_METADATA = {
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment here noting this the the pelicanconf.py file. E.g.

# pelicanconf.py

IMAGE_PROCESS_METADATA = {
# ...the rest

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

if isinstance(value, str) and key in metadata_to_process:
derivative = generator.context["IMAGE_PROCESS_METADATA"][key]
# If value starts with {some-other-derivative}, override derivative
if value.startswith("{") and "}" in value:
Copy link
Member

Choose a reason for hiding this comment

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

I think we should we add a guard against using "keys" such as {attach} or {static} that have special meaning already to Pelican. I think the list can be imported from pelican, which would be ideal rather than maintaining such a list here manually.

Probably best to throw a warning if one of the disallowed keys is encountered.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not realize that I was using the same syntax as the {static} and {attach} feature. I think I should use a different syntax to avoid conflicts. I could use [transform-name] instead. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

It's hard to find something that isn't in use somewhere... for example, the [] reminds me of (the first half of) a Markdown link, and I also worry that it might get interpreted as a list if someone is trying to process the post metadata as YAML.

I think the {} is okay, as it already feels "Pelican-y" (exactly because it is used elsewhere). One option would be to "namespace" it; i.e. require that all keys start with the "image-process" namespace. E.g. {image-process-transform-name}. The trade-off is it gets kind of verbose.

Or just blacklist the keys that Pelican is already using.

I think the list can be imported from pelican,

I went looking for this list tonight and can't find it :(

Copy link
Collaborator Author

@patrickfournier patrickfournier Nov 17, 2025

Choose a reason for hiding this comment

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

I added a check in the code and a test case.

@minchinweb
Copy link
Member

Also, which is prefer? For me to make suggestions like this for documentation, or just write it myself and push it to the PR and ask for your review?

@patrickfournier
Copy link
Collaborator Author

@minchinweb Thanks for the comments. Unless it is something very minor, I prefer suggestions than additional commits, because we can discuss them (if there is a need for discussion) and I feel it is less work for the reviewer (no need to setup a dev environment, no need to test the suggestions, no need to lint the files, ...).

- Clarify the feature documentation in README.md.
- Ignore Pelican special linking directives.
- Add a test for Pelican special linking directives.
Copy link
Member

@minchinweb minchinweb left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this!

@justinmayer justinmayer changed the title Process metadata Process images in content metadata Nov 18, 2025
Copy link
Contributor

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Many thanks to @patrickfournier for the significant enhancement and to @minchinweb for reviewing! 🏅

@justinmayer justinmayer merged commit 29bafa9 into pelican-plugins:main Nov 18, 2025
12 checks passed
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.

3 participants