From 33719165ff1708a04f7f84afcd9e17544eec27e9 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Fri, 29 Nov 2024 11:45:37 +0100 Subject: [PATCH 1/4] Create linter and allowlist --- tools/verify_images.py | 109 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tools/verify_images.py diff --git a/tools/verify_images.py b/tools/verify_images.py new file mode 100644 index 0000000..2930e04 --- /dev/null +++ b/tools/verify_images.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +# This code is part of Qiskit. +# +# (C) Copyright IBM 2024 +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Utility script to verify that all images have alt text""" + +from pathlib import Path +import multiprocessing +import sys +import glob + +# Dictionary to allowlist lines of code that the checker will not error +# Format: {"file_path": [list_of_line_numbers]} +ALLOWLIST_MISSING_ALT_TEXT = { + "qiskit_addon_obp/utils/visualization.py": [38, 101, 148, 203, 252, 306, 362] +} + + +def is_image(line: str) -> bool: + return line.strip().startswith((".. image:", ".. plot:")) + + +def is_option(line: str) -> bool: + return line.strip().startswith(":") + +def in_allowlist(filename: str, line_num: int) -> bool: + return line_num in ALLOWLIST_MISSING_ALT_TEXT.get(filename, []) + + +def validate_image(file_path: str) -> tuple[str, list[str]]: + """Validate all the images of a single file""" + invalid_images: list[str] = [] + + lines = Path(file_path).read_text().splitlines() + + line_index = 0 + image_found = False + image_line = -1 + options: list[str] = [] + + while line_index < len(lines): + line = lines[line_index].strip() + + if image_found and not is_option(line) and not is_valid_image(options): + invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") + image_found = False + options = [] + continue + + if image_found and is_option(line): + options.append(line) + + if is_image(line) and not in_allowlist(file_path, line_index + 1): + image_found = True + image_line = line_index + 1 + options = [] + + line_index += 1 + + return (file_path, invalid_images) + + +def is_valid_image(options: list[str]) -> bool: + alt_exists = any(option.startswith(":alt:") for option in options) + nofigs_exists = any(option.startswith(":nofigs:") for option in options) + + # Only `.. plot::`` directives without the `:nofigs:` option are required to have alt text. + # Meanwhile, all `.. image::` directives need alt text and they don't have a `:nofigs:` option. + return alt_exists or nofigs_exists + +def main() -> None: + files = glob.glob("qiskit_addon_obp/**/*.py", recursive=True) + + with multiprocessing.Pool() as pool: + results = pool.map(validate_image, files) + + failed_files = [x for x in results if len(x[1])] + + if not len(failed_files): + print("✅ All images have alt text") + sys.exit(0) + + print("💔 Some images are missing the alt text", file=sys.stderr) + + for filename, image_errors in failed_files: + print(f"\nErrors found in {filename}:", file=sys.stderr) + + for image_error in image_errors: + print(image_error, file=sys.stderr) + + print( + "\nAlt text is crucial for making documentation accessible to all users. It should serve the same purpose as the images on the page, conveying the same meaning rather than describing visual characteristics. When an image contains words that are important to understanding the content, the alt text should include those words as well.", + file=sys.stderr, + ) + + sys.exit(1) + + +if __name__ == "__main__": + main() \ No newline at end of file From e7a888b2636ebde1a804baa84ce84131025749c0 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Fri, 29 Nov 2024 18:18:50 +0100 Subject: [PATCH 2/4] refactor linter and add it to CI --- tools/verify_images.py | 56 ++++++++++++++++++------------------------ tox.ini | 1 + 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/tools/verify_images.py b/tools/verify_images.py index 2930e04..64214c8 100644 --- a/tools/verify_images.py +++ b/tools/verify_images.py @@ -18,11 +18,10 @@ import sys import glob -# Dictionary to allowlist lines of code that the checker will not error -# Format: {"file_path": [list_of_line_numbers]} -ALLOWLIST_MISSING_ALT_TEXT = { - "qiskit_addon_obp/utils/visualization.py": [38, 101, 148, 203, 252, 306, 362] -} +# List of allowlist files that the checker will not verify +ALLOWLIST_MISSING_ALT_TEXT = [ + "qiskit_addon_obp/utils/visualization.py" +] def is_image(line: str) -> bool: @@ -32,51 +31,44 @@ def is_image(line: str) -> bool: def is_option(line: str) -> bool: return line.strip().startswith(":") -def in_allowlist(filename: str, line_num: int) -> bool: - return line_num in ALLOWLIST_MISSING_ALT_TEXT.get(filename, []) + +def is_valid_image(options: list[str]) -> bool: + alt_exists = any(option.strip().startswith(":alt:") for option in options) + nofigs_exists = any(option.strip().startswith(":nofigs:") for option in options) + + # Only `.. plot::`` directives without the `:nofigs:` option are required to have alt text. + # Meanwhile, all `.. image::` directives need alt text and they don't have a `:nofigs:` option. + return alt_exists or nofigs_exists def validate_image(file_path: str) -> tuple[str, list[str]]: """Validate all the images of a single file""" + + if file_path in ALLOWLIST_MISSING_ALT_TEXT: + return [file_path, []] + invalid_images: list[str] = [] lines = Path(file_path).read_text().splitlines() - line_index = 0 image_found = False - image_line = -1 options: list[str] = [] - while line_index < len(lines): - line = lines[line_index].strip() - - if image_found and not is_option(line) and not is_valid_image(options): - invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") - image_found = False - options = [] - continue - + for line_index, line in enumerate(lines): if image_found and is_option(line): options.append(line) + continue - if is_image(line) and not in_allowlist(file_path, line_index + 1): - image_found = True - image_line = line_index + 1 - options = [] + if image_found and not is_valid_image(options): + image_line = line_index - len(options) + invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") - line_index += 1 + image_found = is_image(line) + options = [] return (file_path, invalid_images) -def is_valid_image(options: list[str]) -> bool: - alt_exists = any(option.startswith(":alt:") for option in options) - nofigs_exists = any(option.startswith(":nofigs:") for option in options) - - # Only `.. plot::`` directives without the `:nofigs:` option are required to have alt text. - # Meanwhile, all `.. image::` directives need alt text and they don't have a `:nofigs:` option. - return alt_exists or nofigs_exists - def main() -> None: files = glob.glob("qiskit_addon_obp/**/*.py", recursive=True) @@ -106,4 +98,4 @@ def main() -> None: if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/tox.ini b/tox.ini index 65a6177..2410fe2 100644 --- a/tox.ini +++ b/tox.ini @@ -31,6 +31,7 @@ commands = nbqa ruff docs/ mypy qiskit_addon_obp/ pylint -rn qiskit_addon_obp/ test/ + python tools/verify_images.py nbqa pylint -rn docs/ typos reno lint From 55b8f86d2c1d5cd7ec2e5ddb53797d5f402c64dd Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Tue, 3 Dec 2024 14:26:36 +0100 Subject: [PATCH 3/4] refactor loop --- tools/verify_images.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tools/verify_images.py b/tools/verify_images.py index 64214c8..34f9396 100644 --- a/tools/verify_images.py +++ b/tools/verify_images.py @@ -55,13 +55,18 @@ def validate_image(file_path: str) -> tuple[str, list[str]]: options: list[str] = [] for line_index, line in enumerate(lines): - if image_found and is_option(line): - options.append(line) - continue - - if image_found and not is_valid_image(options): - image_line = line_index - len(options) - invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") + if image_found: + if is_option(line): + options.append(line) + continue + + # Else, the prior image_found has no more options so we should determine if it was valid. + # + # Note that, either way, we do not early exit out of the loop iteration because this `line` + # might be the start of a new image. + if not is_valid_image(options): + image_line = line_index - len(options) + invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") image_found = is_image(line) options = [] @@ -75,7 +80,7 @@ def main() -> None: with multiprocessing.Pool() as pool: results = pool.map(validate_image, files) - failed_files = [x for x in results if len(x[1])] + failed_files = {file: image_errors for file, image_errors in results if image_errors} if not len(failed_files): print("✅ All images have alt text") @@ -83,8 +88,8 @@ def main() -> None: print("💔 Some images are missing the alt text", file=sys.stderr) - for filename, image_errors in failed_files: - print(f"\nErrors found in {filename}:", file=sys.stderr) + for file, image_errors in failed_files.items(): + print(f"\nErrors found in {file}:", file=sys.stderr) for image_error in image_errors: print(image_error, file=sys.stderr) From 90586b2f1e724aeaa1d4737a8ce00606d4b208b8 Mon Sep 17 00:00:00 2001 From: Arnau Casau Date: Tue, 10 Dec 2024 16:21:36 +0100 Subject: [PATCH 4/4] use VCS to install the linter --- tools/verify_images.py | 106 ----------------------------------------- tox.ini | 5 +- 2 files changed, 4 insertions(+), 107 deletions(-) delete mode 100644 tools/verify_images.py diff --git a/tools/verify_images.py b/tools/verify_images.py deleted file mode 100644 index 34f9396..0000000 --- a/tools/verify_images.py +++ /dev/null @@ -1,106 +0,0 @@ -#!/usr/bin/env python3 -# This code is part of Qiskit. -# -# (C) Copyright IBM 2024 -# -# This code is licensed under the Apache License, Version 2.0. You may -# obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -# -# Any modifications or derivative works of this code must retain this -# copyright notice, and modified files need to carry a notice indicating -# that they have been altered from the originals. - -"""Utility script to verify that all images have alt text""" - -from pathlib import Path -import multiprocessing -import sys -import glob - -# List of allowlist files that the checker will not verify -ALLOWLIST_MISSING_ALT_TEXT = [ - "qiskit_addon_obp/utils/visualization.py" -] - - -def is_image(line: str) -> bool: - return line.strip().startswith((".. image:", ".. plot:")) - - -def is_option(line: str) -> bool: - return line.strip().startswith(":") - - -def is_valid_image(options: list[str]) -> bool: - alt_exists = any(option.strip().startswith(":alt:") for option in options) - nofigs_exists = any(option.strip().startswith(":nofigs:") for option in options) - - # Only `.. plot::`` directives without the `:nofigs:` option are required to have alt text. - # Meanwhile, all `.. image::` directives need alt text and they don't have a `:nofigs:` option. - return alt_exists or nofigs_exists - - -def validate_image(file_path: str) -> tuple[str, list[str]]: - """Validate all the images of a single file""" - - if file_path in ALLOWLIST_MISSING_ALT_TEXT: - return [file_path, []] - - invalid_images: list[str] = [] - - lines = Path(file_path).read_text().splitlines() - - image_found = False - options: list[str] = [] - - for line_index, line in enumerate(lines): - if image_found: - if is_option(line): - options.append(line) - continue - - # Else, the prior image_found has no more options so we should determine if it was valid. - # - # Note that, either way, we do not early exit out of the loop iteration because this `line` - # might be the start of a new image. - if not is_valid_image(options): - image_line = line_index - len(options) - invalid_images.append(f"- Error in line {image_line}: {lines[image_line-1].strip()}") - - image_found = is_image(line) - options = [] - - return (file_path, invalid_images) - - -def main() -> None: - files = glob.glob("qiskit_addon_obp/**/*.py", recursive=True) - - with multiprocessing.Pool() as pool: - results = pool.map(validate_image, files) - - failed_files = {file: image_errors for file, image_errors in results if image_errors} - - if not len(failed_files): - print("✅ All images have alt text") - sys.exit(0) - - print("💔 Some images are missing the alt text", file=sys.stderr) - - for file, image_errors in failed_files.items(): - print(f"\nErrors found in {file}:", file=sys.stderr) - - for image_error in image_errors: - print(image_error, file=sys.stderr) - - print( - "\nAlt text is crucial for making documentation accessible to all users. It should serve the same purpose as the images on the page, conveying the same meaning rather than describing visual characteristics. When an image contains words that are important to understanding the content, the alt text should include those words as well.", - file=sys.stderr, - ) - - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/tox.ini b/tox.ini index 2410fe2..f4dcff8 100644 --- a/tox.ini +++ b/tox.ini @@ -21,6 +21,9 @@ commands = nbqa ruff --fix docs/ [testenv:lint] +image-tester-commit = 7ae965ccf21c39e5170334ec7f4882756883860a +deps = + git+https://github.com/Qiskit/documentation.git@{[testenv:lint]image-tester-commit}\#egg=sphinx-alt-text-validator&subdirectory=scripts/image-tester basepython = python3.10 extras = lint @@ -31,7 +34,7 @@ commands = nbqa ruff docs/ mypy qiskit_addon_obp/ pylint -rn qiskit_addon_obp/ test/ - python tools/verify_images.py + sphinx-alt-text-validator -f qiskit_addon_obp -s qiskit_addon_obp/utils/visualization.py nbqa pylint -rn docs/ typos reno lint