Skip to content

Use source hashes for cover matching #730

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

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

mizaki
Copy link
Contributor

@mizaki mizaki commented Feb 11, 2025

Is this the kind of thing you had in mind?

This turned out quite simple.

@mizaki
Copy link
Contributor Author

mizaki commented Feb 11, 2025

Metron talker patch:

Subject: [PATCH] Use image hash
---
Index: metron_talker/metron.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/metron_talker/metron.py b/metron_talker/metron.py
--- a/metron_talker/metron.py	(revision 4e0d5c134d65ac4aa908a1cbd45a71cc6846d2c7)
+++ b/metron_talker/metron.py	(date 1739233743602)
@@ -31,7 +31,7 @@
 
 import settngs
 from comicapi import utils
-from comicapi.genericmetadata import ComicSeries, GenericMetadata, MetadataOrigin
+from comicapi.genericmetadata import ComicSeries, GenericMetadata, MetadataOrigin, ImageHash
 from comicapi.issuestring import IssueString
 from comicapi.utils import LocationParseError, parse_url
 from comictalker import talker_utils
@@ -1022,7 +1022,8 @@
             elif series["year_began"]:
                 md.year = utils.xlate_int(series["year_began"])
 
-        md._cover_image = issue["image"]
+        # md._cover_image = issue["image"]
+        md._cover_image = ImageHash(Hash=int(issue["cover_hash"], 16), Kind="phash")
 
         if issue.get("alt_number"):
             md.alternate_number = issue["alt_number"]

Copy link
Member

@lordwelch lordwelch left a comment

Choose a reason for hiding this comment

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

Yes, this looks good

@mizaki
Copy link
Contributor Author

mizaki commented Feb 12, 2025

I don't think it's going to be much more effort to put a cover_hashes and alt_covers_hashes into GenericMetadata and then it's all set up and ready to go into the future? (And also, the issue cover images are broken in the issue window which there isn't a real way around if the url has been replaced.)

Only real question is representation of the GM field: tuple[ImageHash] or {phash: <hash>, ahash: <hash>}?

@lordwelch
Copy link
Member

I don't think it's going to be much more effort to put a cover_hashes and alt_covers_hashes into GenericMetadata and then it's all set up and ready to go into the future?

Lets keep it as is for now. We can look at other ways of doing it later

(And also, the issue cover images are broken in the issue window which there isn't a real way around if the url has been replaced.)

#730 (comment) should take care of any unexpected errors, they should already handle an empty url (or the metron talker shouldn't work there at all).

@mizaki
Copy link
Contributor Author

mizaki commented Feb 12, 2025

I don't think it's going to be much more effort to put a cover_hashes and alt_covers_hashes into GenericMetadata and then it's all set up and ready to go into the future?

Lets keep it as is for now. We can look at other ways of doing it later

I've added URL to ImageHash which seems like the simplest options atm and solves the problems.

If you're happy, the hard part next is making the tests :)

@lordwelch
Copy link
Member

It's simpler to just explicitly ignore the ImageHash outside of the IssueIdentifier

@lordwelch lordwelch closed this Feb 24, 2025
@lordwelch lordwelch reopened this Feb 24, 2025
@@ -639,7 +639,7 @@ def save(self, ca: ComicArchive, match_results: OnlineMatchResults) -> tuple[Res
month=ct_md.month,
year=ct_md.year,
publisher=None,
image_url=ct_md._cover_image or "",
image_url=str(ct_md._cover_image) or "",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this okay as is or should it be ct_md._cover_image if isinstance(issue._cover_image, str) else "" as well?

@mizaki
Copy link
Contributor Author

mizaki commented Feb 24, 2025

I thought adding the URL to ImageHash covered all the bases and at a later date it could be used as a fallback if wanted. But I can see that not being wanted anyway.

Is there anything you want me to do with this?

@lordwelch lordwelch merged commit 8837fea into comictagger:develop Feb 27, 2025
6 of 9 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.

2 participants