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

allow matcher graph to cope with suppressed images (take 2) #2539

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

paul-butcher
Copy link
Contributor

What does this change?

The current incarnation of the matcher graph script cannot cope with graphs containing suppressed images. This fixes that problem.

The problem has two causes:

  1. The ".jpg" file that gets downloaded in this situation is actually a JSON description of the error
    • This is fixed by copying in a placeholder "No Image" image if the response is unsuccessful
  2. html labels in .dot files can be either a table or some text, not both. When generating the label for a suppressed image, we were appending "(suppressed)" to a table containing the image.
    • I have fixed this by appending "(suppressed)" to the text part of the label, rather than the whole thing.

I noticed this whilst trying to diagnose #2536

How to test

Run the matcher graph generator on a graph that contains a suppressed image, e.g. yarn getMatcherGraph szjj6gqy

Before this change, you would have seen an error complaining about a surprise (

After this change, you will see this:
szjj6gqy.pdf

How can we measure success?

We'll be able to more reliably generate matcher graphs to help diagnose potential merge problems.

Have we considered potential risks?

This is an internal tool used by developers to help diagnose problems. It does not interfere with the running of anything. The only risk I can imagine is if we genuinely have an image in the collection that looks like the placeholder image, and someone gets confused. I think that highly unlikely.

@agnesgaroux
Copy link
Contributor

Tried it, all good

Querying catalogue-2024-01-09_works-graph for szjj6gqy
not ok
_images/MiroImageNumber/L0005170.jpg
https://iiif.wellcomecollection.org/thumbs/L0005170/full/!100,100/0/default.jpg
Error Found: null
Written _graphs/szjj6gqy.pdf

@paul-butcher paul-butcher merged commit 56187b9 into main Jan 26, 2024
1 check passed
@paul-butcher paul-butcher deleted the matcher-graph-2 branch January 26, 2024 15:50
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