verify: Keep one draft per source, not per directory (t42)

Author: peterbjohnson

CHANGELOG.md

@@ -13,6 +13,7 @@
 - A field quoted out of a list item is now dedented as commonmark reads the item: the marker comes off the first line and as much of the same width off every line under it. So a question written `1.  ` no longer carries its number, a continuation line no longer arrives indented far enough to be rendered as a code block, and a spec's `strip` is left with what pandoc does not read as a marker. Values written by `in2lambda spec run`, `in2lambda draft question add`, `in2lambda draft part add` and `in2lambda draft question solution` change accordingly; the ranges behind them still name the same source lines.
 - `in2lambda validate` checks a draft over as a whole and writes what it finds into it as a `report`: source blocks in no field and not marked ignore, two fields taken from the same lines, gaps in the numbering of the questions or their parts, and fields holding nothing, each at level `error`; a part, or a question written without parts, that nothing in the draft answers is reported at level `warning` instead. Each finding names the level, the field and the lines it is about, so it can be acted on without reading the draft. Finding something is not a failure and the command still exits 0; the report is replaced by the next run of the checks and dropped by the next command that changes the draft, since it describes the draft as it stood. It also checks over the set the draft describes, as a converted document is checked at export - maths delimiters, what KaTeX will not render, images the export would not carry, and the compile Lambda Feedback's PDF generator does where pandoc and xelatex are installed, with a warning saying what to install where they are not - and reports each of those against the draft field the text is written in, at level `error`, so that `in2lambda build` refuses them as it refuses anything else at that level.
 - `in2lambda build` writes a draft out as a Lambda Feedback set: one question per `qN.text` field, holding the parts written for it and the worked solutions, with the images those fields refer to under `media/`, as `in2lambda convert` writes a set - a field naming an image that is not beside the draft is refused saying which file is missing, since the checks read the draft and not the folder it is in, and a question's own solution written beside a solution for every part it has becomes a part of its own holding just that solution, as `convert` pairs them up. It is refused unless `in2lambda validate` has been run since the draft last changed - every command that changes one drops its report - and found nothing at level `error`, and the refusal prints those findings so they can be acted on without opening the draft. A finding at level `warning` - a part or question nothing in the draft answers - does not stop it: half the sheets there are keep their solutions in another file or have none at all, so the warning is printed and the set written all the same, rather than a solution having to be invented to quiet it. `in2lambda render` writes each question as a PDF instead, compiled as Lambda Feedback's own PDF generator compiles it, which needs pandoc and xelatex; it is gated on nothing, since looking at a draft is how what the checks found gets fixed. Both take `-o/--out`, as `convert` does.
+- An export now names its images as they sit in `media/`: every markdown image reference a question holds - in its text, a part's, a worked solution, a final answer or an answer box's wording - is rewritten to the file name the image was carried under, so a document writing `![](figures/train.png)` exports as `![](train.png)` beside `media/train.png` and Lambda Feedback finds the figure where it looks for one. A file two questions use is carried once; where two different files are called the same, the second is named as Lambda Feedback's own exports name an image, `question_001_<Title>_0001.png`. Reading an export back is unchanged, since an export already names its images this way.
 - Every command that works on a draft - `in2lambda source show`, each of the `in2lambda draft` commands, `in2lambda spec run`, `in2lambda validate`, `in2lambda build` and `in2lambda render` - takes `--draft`, naming either the draft or the source it was frozen from. Left off, it uses the one draft in the current directory, and where there is more than one it is refused naming them rather than acting on whichever sorts first. `in2lambda spec run` names its SPEC from the draft's directory. The Python functions behind them take the draft's path rather than a directory: `in2lambda.source.frozen`, `in2lambda.source.show`, `in2lambda.draft.execute`, `in2lambda.draft.replay`, `in2lambda.draft.spec_command`, `in2lambda.draft.report.validate`, `in2lambda.draft.export.build` and `in2lambda.draft.export.render`. `in2lambda.source.draft_of` says where a document's draft goes and `in2lambda.source.find` is what the command line resolves `--draft` with.
 - Importing `in2lambda.katex_convert` no longer writes a file called `log` into the working directory. What it has to say about a converted expression goes to the `in2lambda.katex_convert` logger, which is silent unless the application configures logging.
 - The Python API is unchanged: `in2lambda.main.runner` and everything under `in2lambda.api` take the same arguments and return the same objects.

docs/source/filters.py

@@ -37,14 +37,18 @@ def generate_filters_docs():
         pdf_file = f"../../{'../' if os.getenv('GITHUB_ACTIONS') == 'true' else './'}{static_pdf_directory}/{filter_name}.pdf"
 
         if shutil.which("pdflatex"):
+            example = Path(filter_module.__file__).parent / "example.tex"
             subprocess.run(
                 [
                     "pdflatex",
-                    f"-output-directory={static_pdf_directory}",
+                    f"-output-directory={static_pdf_directory.resolve()}",
                     f"-jobname={filter_name}",
                     "-interaction=nonstopmode",
-                    tex_file,
+                    example.name,
                 ],
+                # An example naming a figure names it as it sits beside the document,
+                # so the document is compiled from its own directory.
+                cwd=example.parent,
                 check=True,
             )
 

docs/source/question-format.md

@@ -100,8 +100,9 @@ A few things the example shows in passing:
 - **Unset question settings are left out of the JSON** rather than guessed at, so `skill`,
   `guidance` and the two durations only appear when set. `publish` and the four `display_*`
   settings always do, defaulting to `True`.
-- **Images** go in `Question.images` as paths on disk; they are copied into `media/` keeping the
-  file name they already had, and referred to from the markdown by that name.
+- **Images** go in `Question.images` as paths on disk; they are copied into `media/` under the file
+  name they already had, and every reference to one in the question's markdown is rewritten to that
+  name, which is all Lambda Feedback looks an image up by.
 - **{meth}`Set.from_json <in2lambda.api.set.Set.from_json>`** reads an existing export, as a folder
   or a zip, so an edit to a real set can start from what Lambda Feedback produced.
 
@@ -111,14 +112,16 @@ A few things the example shows in passing:
 <set name>/set_<Name>.json
 <set name>/question_000_<Title>.json          # 000 is the question's orderNumber
 <set name>/question_001_...
-<set name>/media/rocket-momentum.png              # one per Question.images path
+<set name>/media/rocket-momentum.png              # one per file Question.images names
 <set name>.zip                                 # the folder, zipped, to upload
 ```
 
 A question's filename is its title with spaces and the characters Windows and path separators
 forbid (`/ \ < > : " | ?  *`) each replaced by an underscore. An image keeps the file name it
-already had, so `images=["figures/rocket-momentum.png"]` gives `media/rocket-momentum.png`.
-Files are written on a single line.
+already had, so `images=["figures/rocket-momentum.png"]` gives `media/rocket-momentum.png`, and the
+references to it are rewritten to that name. `media/` is one flat folder for the whole set, so a
+file two questions use is copied once, and a second file of a name already taken is named as Lambda
+Feedback names one, `question_001_<Title>_0001.png`. Files are written on a single line.
 
 ### Set
 
@@ -170,9 +173,11 @@ An `id` left unset is a fresh UUID, which is what import needs.
 Maths is `$...$` inline and `$$` on its own lines for display, rendered by
 [KaTeX](https://katex.org/): commands KaTeX lacks do not display — degrees, for example, are
 written `^\circ`. An image is written `![pictureTag](rocket-momentum.png)`, naming the file as it
-sits in `media/`. A filter instead passes through whatever path the source document used, so
+sits in `media/`. A filter passes through whatever path the source document used, so
 `\includegraphics{figures/rocket-momentum.png}` becomes `![pictureTag](figures/rocket-momentum.png)`
-beside `media/rocket-momentum.png`.
+in the set; writing the set out rewrites it to `![pictureTag](rocket-momentum.png)`, which is the
+image as `media/` holds it. A reference naming no image of the question is left as written, and
+{func}`~in2lambda.validation.validate` reports it.
 
 :::{note}
 Lambda Feedback's own exports carry a few keys in2lambda neither reads nor writes, among them

in2lambda/draft/export.py

@@ -23,9 +23,9 @@
 from in2lambda.api.part import Part
 from in2lambda.api.question import Question
 from in2lambda.api.set import Set
-from in2lambda.json_convert.json_convert import _question_stem, _question_title
+from in2lambda.json_convert.json_convert import _IMAGE, _question_stem, _question_title
 from in2lambda.source import ConversionToolsMissing, SourceError, frozen
-from in2lambda.validation import _IMAGE, _location, pdf
+from in2lambda.validation import _location, pdf
 
 _QUESTION = re.compile(r"q(\d+)\.text")
 """A question's text, and the number that orders it."""

in2lambda/filters/PartsOneSol/example.tex

@@ -1,6 +1,7 @@
 \documentclass[12pt]{article}
 
 \usepackage{comment}
+\usepackage{graphicx}
 
 % This is a common method for including/excluding solutions from the PDF.
 \includecomment{solution}
@@ -15,7 +16,7 @@ \subsection{}
 Here is some preliminary question information that might be useful.
 
 \begin{enumerate}
-    \item This is part (a)
+    \item This is part (a), and the apparatus is shown in \includegraphics{./figures/pistons.png}
     \item The filter still works even if there aren't any parts
 \end{enumerate}
 

in2lambda/filters/PartsOneSol/figures/pistons.png

@@ -8,7 +8,7 @@
 import zipfile
 from copy import deepcopy
 from pathlib import Path
-from typing import Any
+from typing import Any, Optional
 
 from in2lambda.api.part import Part
 from in2lambda.api.question import Question
@@ -19,6 +19,33 @@
 MINIMAL_QUESTION_TEMPLATE = "minimal_template_question.json"
 MINIMAL_SET_TEMPLATE = "minimal_template_set.json"
 
+_IMAGE = re.compile(r"!\[[^\]]*\]\(([^)]*)\)")
+"""A markdown image, e.g. ``![pictureTag](question_000_Title_0001.png)``."""
+
+
+def _image_for(reference: str, images: list[str]) -> Optional[str]:
+    """Which of a question's images a markdown reference names, if any.
+
+    Matched by file name, because that is the link between the two: a filter resolves
+    the very path it leaves in the markdown, and Lambda Feedback finds an image in
+    ``media/`` by its file name alone. Only where a question lists two files of the same
+    name does the rest of the reference decide, by naming the end of one of their paths.
+
+    Returns:
+        The image, or None if the question lists none of that name - in which case the
+        reference is left as written, which :mod:`in2lambda.validation` reports.
+    """
+    named = [image for image in images if Path(image).name == Path(reference).name]
+    if len(named) > 1:
+        # A filter keeps the reference as the document wrote it but normalises the path
+        # it lists beside it, so a `..` is present on one side only and has to come off
+        # for the two to line up. A `.` is already gone, dropped by ``pathlib``.
+        parts = tuple(part for part in Path(reference).parts if part != "..")
+        named = [
+            image for image in named if Path(image).parts[-len(parts) :] == parts
+        ] or named
+    return named[0] if named else None
+
 
 def _templates() -> tuple[dict[str, Any], dict[str, Any]]:
     """Loads the minimal question and set templates that the writer fills in.
@@ -47,9 +74,8 @@ def _zip(files: list[Path], root: Path, zip_path: str) -> None:
         root: The folder the archive names are relative to.
         zip_path: The path where the zip file will be created.
     """
-    # Sort by archive name for deterministic, alphabetical order. A file can be
-    # written more than once — an image used by both a question and its worked
-    # solution — and is still one file on disk, so name it once here too.
+    # Sort by archive name for deterministic, alphabetical order, and name each file
+    # once: a file written twice is still one file on disk.
     names = sorted({str(file.relative_to(root)): file for file in files}.items())
     with zipfile.ZipFile(zip_path, "w") as zf:
         for name, file in names:
@@ -226,8 +252,55 @@ def _question_json(
     return output
 
 
+def _media_name(image: str, stem: str, taken: set[str]) -> str:
+    """What an image is called in ``media/``, which is flat and so has one of each name.
+
+    Its own file name, or, where that name is another file's already, the name Lambda
+    Feedback's own exports give an image: the question's, numbered.
+    """
+    name = Path(image).name
+    if name not in taken:
+        return name
+    number = 1
+    while (numbered := f"{stem}_{number:04}{Path(image).suffix}") in taken:
+        number += 1
+    return numbered
+
+
+def _with_media_names(value: Any, question: Question, media: dict[str, str]) -> Any:
+    """A question's JSON with every image reference in it rewritten to its media name.
+
+    Walked rather than taken field by field because a reference can be written in any
+    markdown the question holds - its text, a part's, a worked solution, a final answer,
+    an answer box's wording or one of its options - and a second list of those here would
+    drift from the one :mod:`in2lambda.validation` already checks.
+    """
+    if isinstance(value, dict):
+        return {
+            key: _with_media_names(item, question, media) for key, item in value.items()
+        }
+    if isinstance(value, list):
+        return [_with_media_names(item, question, media) for item in value]
+    if not isinstance(value, str):
+        return value
+
+    def rewrite(reference: re.Match[str]) -> str:
+        image = _image_for(reference[1], question.images)
+        if image is None:
+            return reference[0]
+        # Only the path is replaced; the alt text beside it may well read the same.
+        name = media[os.path.abspath(image)]
+        return reference[0][: reference.start(1) - reference.start()] + name + ")"
+
+    return _IMAGE.sub(rewrite, value)
+
+
 def _write_question(
-    question: Question, i: int, template: dict[str, Any], folder: Path
+    question: Question,
+    i: int,
+    template: dict[str, Any],
+    folder: Path,
+    media: dict[str, str],
 ) -> list[Path]:
     """Writes one question's JSON, and any images it uses, into an existing folder.
 
@@ -236,25 +309,32 @@ def _write_question(
         i: Its order number, which also prefixes the file name.
         template: The loaded JSON from the minimal question template.
         folder: The folder to write into.
+        media: What the export has carried into ``media/`` so far, each image's path on
+            disk against the name it was written under. Added to as this question's
+            images are copied, so that a file two questions use is one file under one
+            name.
 
     Returns:
         The files written.
     """
     output = _question_json(question, i, template)
+    stem = _question_stem(i, output["title"])
 
-    json_file = folder / f"{_question_stem(i, output['title'])}.json"
-    with open(json_file, "w") as file:
-        json.dump(output, file)
-    written = [json_file]
-
+    written = []
     for image in question.images:
-        # If images exist, create a media directory
-        media = folder / "media"
-        media.mkdir(exist_ok=True)
-        # The JSON refers to an image by its file name, so copying keeps that name.
-        written.append(Path(shutil.copy(os.path.abspath(image), media)))
+        path = os.path.abspath(image)
+        if path in media:
+            continue
+        media[path] = _media_name(path, stem, set(media.values()))
+        # Only a question with an image gets a media folder at all.
+        (folder / "media").mkdir(exist_ok=True)
+        written.append(Path(shutil.copy(path, folder / "media" / media[path])))
+
+    json_file = folder / f"{stem}.json"
+    with open(json_file, "w") as file:
+        json.dump(_with_media_names(output, question, media), file)
 
-    return written
+    return [json_file] + written
 
 
 def write_question(question: Question, output_dir: str, number: int = 0) -> None:
@@ -275,7 +355,7 @@ def write_question(question: Question, output_dir: str, number: int = 0) -> None
         number, _question_title(question, number)
     )
     folder.mkdir(parents=True, exist_ok=True)
-    written = _write_question(question, number, question_template, folder)
+    written = _write_question(question, number, question_template, folder, {})
     _zip(written, folder, f"{folder}.zip")
 
 
@@ -320,8 +400,10 @@ def converter(
         json.dump(set_template, file)
 
     written = [set_file]
+    # Named across the whole set, since media/ is one folder for all of its questions.
+    media: dict[str, str] = {}
     for i, question in enumerate(ListQuestions):
-        written += _write_question(question, i, question_template, folder)
+        written += _write_question(question, i, question_template, folder, media)
 
     # output zip file in destination folder
     _zip(written, folder, output_question + ".zip")

in2lambda/json_convert/json_convert.py

@@ -110,7 +110,7 @@ def runner(
         >>> runner(f"{os.path.dirname(in2lambda.__file__)}/filters/PartsSepSol/example.tex", "PartsSepSol") # doctest: +ELLIPSIS
         Set(_name='set', _description='', _finalAnswerVisibility='OPEN_WITH_WARNINGS', _workedSolutionVisibility='OPEN_WITH_WARNINGS', _structuredTutorialVisibility='OPEN', questions=[Question(title='', parts=[Part(text=..., worked_solution='', answer='', response_areas=[]), ...], images=[], main_text='This is a sample question\n\n'), ...])
         >>> runner(f"{os.path.dirname(in2lambda.__file__)}/filters/PartsOneSol/example.tex", "PartsOneSol") # doctest: +ELLIPSIS
-        Set(_name='set', _description='', _finalAnswerVisibility='OPEN_WITH_WARNINGS', _workedSolutionVisibility='OPEN_WITH_WARNINGS', _structuredTutorialVisibility='OPEN', questions=[Question(title='', parts=[Part(text=..., worked_solution='', answer='', response_areas=[]), ...], images=[], main_text='Here is some preliminary question information that might be useful.'), ...])
+        Set(_name='set', _description='', _finalAnswerVisibility='OPEN_WITH_WARNINGS', _workedSolutionVisibility='OPEN_WITH_WARNINGS', _structuredTutorialVisibility='OPEN', questions=[Question(title='', parts=[Part(text=..., worked_solution='', answer='', response_areas=[]), ...], images=[...], main_text='Here is some preliminary question information that might be useful.'), ...])
     """
     _require_conversion_tools()
     import panflute as pf