From 93d7ff6d7ffda2903daf4823b37b79f10e16cb0d Mon Sep 17 00:00:00 2001 From: Arnau Casau <47946624+arnaucasau@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:34:50 +0100 Subject: [PATCH] Extend the images' checker to check API docs (#2426) This PR extends the images' checker introduced in https://github.com/Qiskit/documentation/pull/2349 to check the API docs when the new argument `--apis` is set. --------- Co-authored-by: Frank Harkins --- .github/workflows/main.yml | 2 +- README.md | 4 ++ scripts/js/commands/checkImages.ts | 56 +++++++++++++++++++++++++-- scripts/js/lib/markdownImages.test.ts | 3 ++ scripts/js/lib/markdownImages.ts | 6 ++- 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c0a821f10b5..facdfd85105 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,7 +29,7 @@ jobs: - name: File metadata run: npm run check:metadata - name: Check images - run: npm run check:images + run: npm run check:images -- --apis - name: Spellcheck run: npm run check:spelling - name: Check Qiskit bot config diff --git a/README.md b/README.md index 90095859fd7..1310a38b3c8 100644 --- a/README.md +++ b/README.md @@ -299,6 +299,10 @@ You can check it locally by running: # Only check images npm run check:images +# By default, only the non-API docs are checked. You can add the +# below argument to also check API docs. +npm run check:images -- --apis + # Or, run all the checks npm run check ``` diff --git a/scripts/js/commands/checkImages.ts b/scripts/js/commands/checkImages.ts index 3b410600e4c..46909871454 100644 --- a/scripts/js/commands/checkImages.ts +++ b/scripts/js/commands/checkImages.ts @@ -11,11 +11,43 @@ // that they have been altered from the originals. import { globby } from "globby"; +import yargs from "yargs/yargs"; +import { hideBin } from "yargs/helpers"; import { collectInvalidImageErrors } from "../lib/markdownImages.js"; import { readMarkdown } from "../lib/markdownReader.js"; +// Known APIs containing images without alt text that will be skipped +// by the checker. +const APIS_TO_IGNORE__SHOULD_FIX: string[] = [ + "qiskit", + "qiskit-ibm-runtime", + "qiskit-addon-cutting", + "qiskit-addon-mpf", + "qiskit-addon-obp", + "qiskit-addon-utils", +]; + +interface Arguments { + [x: string]: unknown; + apis: boolean; +} + +const readArgs = (): Arguments => { + return yargs(hideBin(process.argv)) + .version(false) + .option("apis", { + type: "boolean", + default: false, + description: + "Check the images in the current and dev versions of the API docs have alt text.", + }) + .parseSync(); +}; + async function main() { - const files = await globby(["docs/**/*.{ipynb,mdx}", "!docs/api/**/*.mdx"]); + const args = readArgs(); + + const files = await determineTocFiles(args); const fileErrors: string[] = []; for (const file of files) { @@ -24,7 +56,7 @@ async function main() { if (imageErrors.size) { fileErrors.push( - `Error in file '${file}':\n\t- ${[...imageErrors].join("\n\t- ")}`, + `Error in file '${file}':\n\t- ${[...imageErrors].join("\n\t- ")}\n`, ); } } @@ -32,11 +64,29 @@ async function main() { if (fileErrors.length) { fileErrors.forEach((error) => console.log(error)); console.error( - "\nšŸ’” Some images have problems. See https://github.com/Qiskit/documentation#images for instructions.\n", + "šŸ’” Some images have problems. See https://github.com/Qiskit/documentation#images for instructions.\n", ); process.exit(1); } console.log("āœ… All images are valid.\n"); } +async function determineTocFiles(args: Arguments): Promise { + // We always skip historical versions to simplify the code and to have a faster script. + // Even though it is possible for someone to add a new image without alt text to a + // historical version that wasn't in the original release, that's very unlikely. + // If it happens, it would probably be a backport from latest or dev, and the linter in + // the API repo should catch it. + // + // If an image is missed by the API repo's linter, it will still have an alt text defined, + // although it won't be very useful. + const globs = [ + "docs/**/*.{ipynb,mdx}", + args.apis ? "!docs/api/*/([0-9]*)/*.mdx" : "!docs/api/**/*.mdx", + ...APIS_TO_IGNORE__SHOULD_FIX.map((api) => `!docs/api/${api}/**/*.mdx`), + ]; + + return await globby(globs); +} + main().then(() => process.exit()); diff --git a/scripts/js/lib/markdownImages.test.ts b/scripts/js/lib/markdownImages.test.ts index b885425805b..bb7624da07b 100644 --- a/scripts/js/lib/markdownImages.test.ts +++ b/scripts/js/lib/markdownImages.test.ts @@ -33,6 +33,8 @@ test("Test the finding of invalid images", async () => { Example ![And now, our last link](https://ibm.com) + +![../\_images/invalid\_img3.png](/images/invalid_img3.png) `; const images = await collectInvalidImageErrors(markdown); const correct_images = new Set([ @@ -40,6 +42,7 @@ test("Test the finding of invalid images", async () => { "The image '/images/HTMLexample2.jpg' uses an HTML tag instead of markdown syntax.", "The image '/images/invalid_img1.png' does not have alt text.", "The image '/images/invalid_img2.png' does not have alt text.", + "The image '/images/invalid_img3.png' does not have alt text.", ]); expect(images).toEqual(correct_images); diff --git a/scripts/js/lib/markdownImages.ts b/scripts/js/lib/markdownImages.ts index 590db9b167e..c0698753258 100644 --- a/scripts/js/lib/markdownImages.ts +++ b/scripts/js/lib/markdownImages.ts @@ -17,6 +17,7 @@ import { visit } from "unist-util-visit"; import remarkParse from "remark-parse"; import remarkGfm from "remark-gfm"; import remarkStringify from "remark-stringify"; +import { last, split } from "lodash-es"; export async function collectInvalidImageErrors( markdown: string, @@ -28,7 +29,10 @@ export async function collectInvalidImageErrors( .use(remarkGfm) .use(() => (tree: Root) => { visit(tree, "image", (node) => { - if (!node.alt) { + // Sphinx uses the image path as alt text if it wasn't defined using the + // :alt: option. + const imageName = last(split(node.url, "/")); + if (!node.alt || node.alt.endsWith(imageName!)) { imagesErrors.add(`The image '${node.url}' does not have alt text.`); } });