diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cb18f0..9f9c854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,5 +13,6 @@ - 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 the draft in this directory 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__0001.png`. Reading an export back is unchanged, since an export already names its images this way. - 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. diff --git a/docs/source/filters.py b/docs/source/filters.py index 483896f..7d02c9e 100644 --- a/docs/source/filters.py +++ b/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, ) diff --git a/docs/source/question-format.md b/docs/source/question-format.md index 7d29937..97a3d6c 100644 --- a/docs/source/question-format.md +++ b/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 diff --git a/in2lambda/draft/export.py b/in2lambda/draft/export.py index f077576..dcc5e06 100644 --- a/in2lambda/draft/export.py +++ b/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 DRAFT, 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.""" diff --git a/in2lambda/filters/PartsOneSol/example.tex b/in2lambda/filters/PartsOneSol/example.tex index fbc438f..51cb857 100644 --- a/in2lambda/filters/PartsOneSol/example.tex +++ b/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} diff --git a/in2lambda/filters/PartsOneSol/figures/pistons.png b/in2lambda/filters/PartsOneSol/figures/pistons.png new file mode 100644 index 0000000..bb0e9db Binary files /dev/null and b/in2lambda/filters/PartsOneSol/figures/pistons.png differ diff --git a/in2lambda/json_convert/json_convert.py b/in2lambda/json_convert/json_convert.py index 540143a..503b0ee 100644 --- a/in2lambda/json_convert/json_convert.py +++ b/in2lambda/json_convert/json_convert.py @@ -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") diff --git a/in2lambda/main.py b/in2lambda/main.py index 853aaab..783575d 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.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 diff --git a/in2lambda/validation/__init__.py b/in2lambda/validation/__init__.py index 8b43cd6..97b95bf 100644 --- a/in2lambda/validation/__init__.py +++ b/in2lambda/validation/__init__.py @@ -27,15 +27,13 @@ from in2lambda.api.question import Question from in2lambda.api.response_area import ResponseArea from in2lambda.api.set import Set +from in2lambda.json_convert.json_convert import _IMAGE, _image_for from in2lambda.katex_convert.katex_convert import unsupported_commands from in2lambda.validation import pdf from in2lambda.validation.delimiters import MathDelimiterError, math_delimiter_checker __all__ = ["MathDelimiterError", "Problem", "math_delimiter_checker", "validate"] -_IMAGE = re.compile(r"!\[[^\]]*\]\(([^)]*)\)") -"""A markdown image, e.g. ``![pictureTag](question_000_Title_0001.png)``.""" - _MATHS = re.compile(r"(?<!\\)\$\$(.*?)(?<!\\)\$\$|(?<!\\)\$(.*?)(?<!\\)\$", re.DOTALL) """Display maths first, so that ``$$ ... $$`` is not read as two empty ``$ ... $``.""" @@ -198,10 +196,11 @@ def _markdown_problems( if delimiters is not MathDelimiterError.PASSED: problems.append(Problem(location, delimiters.value)) - # Lambda Feedback finds an image in media/ by its file name alone. - media = {Path(image).name for image in question.images} + # The writer rewrites a reference to the name of the image it matches, and carries + # that image into media/; one it matches nothing for is left as written, which is + # exactly the reference Lambda Feedback will not find. for reference in _IMAGE.findall(markdown): - if Path(reference).name not in media: + if _image_for(reference, question.images) is None: problems.append( Problem(location, f"the export will not contain the image {reference}") ) diff --git a/tests/fixtures/drafts/README.md b/tests/fixtures/drafts/README.md index b6cd92d..9271bb5 100644 --- a/tests/fixtures/drafts/README.md +++ b/tests/fixtures/drafts/README.md @@ -26,7 +26,11 @@ backslash in what the second one writes is written rather than read as a replace anything to say about; an overlap and a gap in the numbering are not here, because no run of commands can make one. `figure_in_a_question` is a question whose text runs on into an image, so the field quotes the reference and `figure.png` beside the source is what the export has to carry -into `media/` - the only folder here with a file the fields point at. +into `media/`. `figure_in_a_folder` is the same question with the figure kept in a `figures/` +folder, as a sheet usually keeps its figures: the field quotes `figures/figure.png`, and the export +carries the file into `media/` and rewrites the reference to the name it is called there, since +that name is all Lambda Feedback looks one up by. Those two are the only folders here with a file +the fields point at. `question_solution_beside_part_solutions` is the only one filled in by a spec rather than by commands one at a time, and the only one whose question has two parts: both are answered by the spec's own solutions, so the `question solution` after it answers nothing, and the export carries diff --git a/tests/fixtures/drafts/figure_in_a_folder/commands.json b/tests/fixtures/drafts/figure_in_a_folder/commands.json new file mode 100644 index 0000000..270cdbd --- /dev/null +++ b/tests/fixtures/drafts/figure_in_a_folder/commands.json @@ -0,0 +1,32 @@ +[ + { + "args": { + "block": "b1" + }, + "by": "tests", + "command": "mark ignore" + }, + { + "args": { + "text": "b2" + }, + "by": "tests", + "command": "question add" + }, + { + "args": { + "question": "q1", + "text": "b3" + }, + "by": "tests", + "command": "part add" + }, + { + "args": { + "question": "q1", + "text": "b4" + }, + "by": "tests", + "command": "question solution" + } +] diff --git a/tests/fixtures/drafts/figure_in_a_folder/expected.json b/tests/fixtures/drafts/figure_in_a_folder/expected.json new file mode 100644 index 0000000..947f3ff --- /dev/null +++ b/tests/fixtures/drafts/figure_in_a_folder/expected.json @@ -0,0 +1,50 @@ +{ + "b1.ignore": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 1, + 1 + ] + ], + "value": true + }, + "q1.p1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 7, + 7 + ] + ], + "value": "Find the load the large piston carries when the small one is pressed with force $f$." + }, + "q1.solution": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 9, + 9 + ] + ], + "value": "The pressure is the same in both, so the load is $F = f A / a$." + }, + "q1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 3, + 5 + ] + ], + "value": "A hydraulic scale has two pistons joined by oil, the small one of area $a$ and the\nlarge one of area $A$.\n![The two pistons](figures/figure.png)" + } +} diff --git a/tests/fixtures/drafts/figure_in_a_folder/figures/figure.png b/tests/fixtures/drafts/figure_in_a_folder/figures/figure.png new file mode 100644 index 0000000..bb0e9db Binary files /dev/null and b/tests/fixtures/drafts/figure_in_a_folder/figures/figure.png differ diff --git a/tests/fixtures/drafts/figure_in_a_folder/source.md b/tests/fixtures/drafts/figure_in_a_folder/source.md new file mode 100644 index 0000000..796ac6f --- /dev/null +++ b/tests/fixtures/drafts/figure_in_a_folder/source.md @@ -0,0 +1,9 @@ +# Hydraulic scale + +A hydraulic scale has two pistons joined by oil, the small one of area $a$ and the +large one of area $A$. +![The two pistons](figures/figure.png) + +Find the load the large piston carries when the small one is pressed with force $f$. + +The pressure is the same in both, so the load is $F = f A / a$. diff --git a/tests/test_draft.py b/tests/test_draft.py index 88fe43f..6d4f00e 100644 --- a/tests/test_draft.py +++ b/tests/test_draft.py @@ -25,8 +25,9 @@ import in2lambda.draft import in2lambda.draft.report from in2lambda.api.set import Set +from in2lambda.json_convert.json_convert import _IMAGE from in2lambda.main import cli -from in2lambda.validation import _IMAGE, pdf +from in2lambda.validation import pdf QUESTION = re.compile(r"q(\d+)\.text") """A question's text among a folder's fields, which is one question of the export.""" @@ -78,6 +79,19 @@ def _expected_parts(fields: dict[str, Any], number: int) -> int: return parts or 1 +def _as_exported(value: str) -> str: + """A field's markdown as the export writes it out. + + The same wording, save that an image reference names the file as it sits in the + export's ``media/``, which is by its own name where no two figures beside the draft + share one. + """ + return _IMAGE.sub( + lambda ref: ref[0][: ref.start(1) - ref.start()] + Path(ref[1]).name + ")", + value, + ) + + def _reported(folder: Path) -> list[dict[str, Any]]: """What the checks should find in a folder's draft; nothing, where it says none.""" report = folder / "report.json" @@ -668,7 +682,7 @@ def test_build_follows_the_report(folder: Path, tmp_path: Path, monkeypatch) -> questions = Set.from_json(str(exported)).questions assert len(questions) == len([key for key in fields if QUESTION.fullmatch(key)]) for number, question in enumerate(questions, start=1): - assert question.main_text == fields[f"q{number}.text"]["value"] + assert question.main_text == _as_exported(fields[f"q{number}.text"]["value"]) # Counted from the fields rather than read off the question, since a loop over # parts that were dropped runs no assertions and passes saying nothing. assert len(question.parts) == _expected_parts(fields, number) @@ -680,25 +694,44 @@ def test_build_follows_the_report(folder: Path, tmp_path: Path, monkeypatch) -> # parts written for it exports as. assert part.text == "" solution = fields.get(f"q{number}.solution") - assert part.worked_solution == (solution["value"] if solution else "") + assert part.worked_solution == ( + _as_exported(solution["value"]) if solution else "" + ) continue - assert part.text == fields[f"q{number}.p{index}.text"]["value"] + assert part.text == _as_exported( + fields[f"q{number}.p{index}.text"]["value"] + ) # A part's own solution, or the question's where it has none of its own. solution = fields.get( f"q{number}.p{index}.solution", fields.get(f"q{number}.solution") ) - assert part.worked_solution == (solution["value"] if solution else "") - - # Every image a field refers to travels with the set under media/, which is the only - # place Lambda Feedback looks for one; the set's folder is asked rather than the - # loaded questions, since reading an export back attributes an image to a question - # by the platform's own naming of the file, which a draft's images do not follow. - assert {path.name for path in (tmp_path / "out" / "set" / "media").glob("*")} == { - Path(reference).name - for field in fields.values() - if isinstance(field["value"], str) - for reference in _IMAGE.findall(field["value"]) + assert part.worked_solution == ( + _as_exported(solution["value"]) if solution else "" + ) + + # Every image a field refers to travels with the set under media/, named as the + # written JSON refers to it, since that name is the only way Lambda Feedback finds + # one; the set's folder is asked rather than the loaded questions, because reading an + # export back attributes an image to a question by the platform's own naming of the + # file, which a draft's images do not follow. + written = tmp_path / "out" / "set" + references = { + reference + for file in written.glob("question_*.json") + for reference in _IMAGE.findall(file.read_text()) } + assert all( + (written / "media" / reference).is_file() for reference in references + ), references + # One file per file the fields point at, whatever path each was written as. + assert len(list((written / "media").glob("*"))) == len( + { + (tmp_path / reference).resolve() + for field in fields.values() + if isinstance(field["value"], str) + for reference in _IMAGE.findall(field["value"]) + } + ) def test_build_refuses_a_field_naming_an_image_that_is_not_there( diff --git a/tests/test_exports.py b/tests/test_exports.py index 105d68d..710bcdd 100644 --- a/tests/test_exports.py +++ b/tests/test_exports.py @@ -214,6 +214,97 @@ def test_repeated_image_zipped_once(tmp_path: Path) -> None: ] +def test_two_figures_of_one_name_are_both_carried(tmp_path: Path) -> None: + """media/ is flat, so the second of two files called the same is named as an export names one.""" + for folder, content in (("a", b"first diagram"), ("b", b"second diagram")): + (tmp_path / folder).mkdir() + (tmp_path / folder / "diagram.png").write_bytes(content) + question_set = Set( + questions=[ + Question( + title=title, + main_text=f"As shown in ![diagram.png]({folder}/diagram.png).", + images=[str(tmp_path / folder / "diagram.png")], + ) + for title, folder in (("First", "a"), ("Second", "b")) + ] + ) + + written = _write_back(question_set, tmp_path) + + assert _relative_files(written / "media") == [ + "diagram.png", + "question_001_Second_0001.png", + ] + assert (written / "media" / "diagram.png").read_bytes() == b"first diagram" + assert [ + json.loads((written / file).read_text())["masterContent"] + for file in ("question_000_First.json", "question_001_Second.json") + ] == [ + "As shown in ![diagram.png](diagram.png).", + "As shown in ![diagram.png](question_001_Second_0001.png).", + ] + + +def test_one_question_telling_two_figures_of_a_name_apart(tmp_path: Path) -> None: + """The only case where which image a reference names is a question, answered by the path.""" + for folder, content in (("a", b"first diagram"), ("b", b"second diagram")): + (tmp_path / folder).mkdir() + (tmp_path / folder / "diagram.png").write_bytes(content) + question_set = Set( + questions=[ + Question( + title="Q", + # A document writes a reference as it sits beside the document, so a + # sheet in a folder of its own climbs out of it to reach the figures, + # and the resolved path listed below has no trace of the climb. + main_text="Before, ![](../a/diagram.png).", + parts=[Part(text="After, ![](../b/diagram.png).")], + images=[ + str(tmp_path / "a" / "diagram.png"), + str(tmp_path / "b" / "diagram.png"), + ], + ) + ] + ) + + written = _write_back(question_set, tmp_path) + + assert _relative_files(written / "media") == [ + "diagram.png", + "question_000_Q_0001.png", + ] + assert (written / "media" / "diagram.png").read_bytes() == b"first diagram" + question = json.loads((written / "question_000_Q.json").read_text()) + assert question["masterContent"] == "Before, ![](diagram.png)." + assert question["parts"][0]["content"] == "After, ![](question_000_Q_0001.png)." + + +def test_one_figure_used_by_two_questions_is_copied_once(tmp_path: Path) -> None: + """Both questions refer to the one file, under the one name it is carried as.""" + image = tmp_path / "figures" / "diagram.png" + image.parent.mkdir() + image.write_bytes(b"not really a png") + question_set = Set( + questions=[ + Question( + title=title, + main_text="As shown in ![diagram.png](figures/diagram.png).", + images=[str(image)], + ) + for title in ("First", "Second") + ] + ) + + written = _write_back(question_set, tmp_path) + + assert _relative_files(written / "media") == ["diagram.png"] + assert [ + json.loads(file.read_text())["masterContent"] + for file in sorted(written.glob("question_*.json")) + ] == ["As shown in ![diagram.png](diagram.png)."] * 2 + + def _area_shape(area: dict) -> frozenset[str]: # Without indices, an area's shape is the keys it has, not how many tests, cases # or symbols it lists. diff --git a/tests/test_runner.py b/tests/test_runner.py index 7881981..a7d30f0 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -14,6 +14,7 @@ from in2lambda.api.set import Set from in2lambda.filters import builtin_filters +from in2lambda.json_convert.json_convert import _IMAGE from in2lambda.main import cli, runner @@ -53,11 +54,17 @@ def test_runner_writes_importable_json( question_files = sorted(set_dir.glob("question_*.json")) assert len(question_files) == len(result.questions) for question_file in question_files: - question_json = json.loads(question_file.read_text()) + written = question_file.read_text() + question_json = json.loads(written) assert question_json["title"] assert "masterContent" in question_json assert "parts" in question_json + # Whatever path the document wrote, the JSON has to name the image as it sits in + # media/, which is the only place Lambda Feedback looks for one. + for reference in _IMAGE.findall(written): + assert (set_dir / "media" / reference).is_file(), reference + def test_cli_reports_problems_and_exports_anyway(tmp_path) -> None: """A problem is printed, and is a warning rather than a refusal to export."""