-
Notifications
You must be signed in to change notification settings - Fork 81
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
Conversation
Metron talker patch:
|
There was a problem hiding this 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
I don't think it's going to be much more effort to put a Only real question is representation of the GM field: |
Lets keep it as is for now. We can look at other ways of doing it later
#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). |
I've added If you're happy, the hard part next is making the tests :) |
14c81d8
to
3d4045a
Compare
3d4045a
to
085b599
Compare
It's simpler to just explicitly ignore the ImageHash outside of the IssueIdentifier |
@@ -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 "", |
There was a problem hiding this comment.
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?
I thought adding the URL to Is there anything you want me to do with this? |
Is this the kind of thing you had in mind?
This turned out quite simple.