diff --git a/CHANGELOG.md b/CHANGELOG.md index 2478fee..fe6a75f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ - `in2lambda draft field replace FIELD OLD NEW` changes the wording inside a field that is already written, for the faults only an edit can fix - a brace the OCR dropped out of some maths, which no range of the source says correctly. OLD has to occur in the field exactly once, or the command is refused saying how many times it occurs; `--regex` reads it as a regular expression and NEW as what to replace it with. The field is left quoting the lines it was taken from, at the layer that wrote it, but recorded as edited and by whoever replaced the wording, so the change can be shown against the source. - `in2lambda spec run SPEC` runs a YAML file of selectors over the frozen source: it says which blocks are questions, parts and solutions, which to ignore, what to strip off the front of each one, and which of the four filters lays the solutions out. It fills in the draft's fields with the markdown of the lines each was taken from, records the spec's name and hash in the log so a replay runs the same file, and reports every block it made nothing of. Running an edited spec over a draft it has already filled in is refused, as freezing a document that has changed is: `in2lambda source add --start-over` begins the draft again. Reading a spec needs pyyaml, which the `convert` extra now installs alongside panflute. See [the spec page](https://lambda-feedback.github.io/in2lambda/spec.html) for the selectors and layouts. - 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, parts nothing answers, and fields holding nothing. Each finding names 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. +- `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, parts nothing answers, and fields holding nothing. Each finding names 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, so that `in2lambda build` refuses them as it refuses anything else in the report. - `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, and the refusal prints what the report says so it can be acted on without opening the draft. `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. - 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/in2lambda/draft/__init__.py b/in2lambda/draft/__init__.py index b603a80..92c6a73 100644 --- a/in2lambda/draft/__init__.py +++ b/in2lambda/draft/__init__.py @@ -16,7 +16,7 @@ from typing import Any import in2lambda.spec -from in2lambda.draft.report import checks, overlapping, uncovered +from in2lambda.draft.report import _order, checks, overlapping, uncovered from in2lambda.source import ( DRAFT, SourceError, @@ -318,9 +318,16 @@ def replay(directory: str = ".") -> None: for entry in draft["log"]: apply(rebuilt, markdown, entry, directory) # The one thing in a draft that no command wrote: the checks did, over the draft the - # commands left, so rebuilding it is running them again rather than copying it. + # commands left, so rebuilding it is running them again rather than copying it. What + # `in2lambda.validation` found over the set is carried across instead, since it + # depends on whether xelatex and Node are installed and the draft does not: rebuilt + # here it would come out shorter on a machine whose toolchain is not the one that + # validated, and an untouched draft would be accused of having been edited. if "report" in draft: - rebuilt["report"] = checks(rebuilt) + carried = [ + finding for finding in draft["report"] if finding["check"] == "problem" + ] + rebuilt["report"] = sorted(checks(rebuilt) + carried, key=_order) path = Path(directory) / DRAFT if serialise(rebuilt) != path.read_bytes(): diff --git a/in2lambda/draft/export.py b/in2lambda/draft/export.py index 911fb0f..5e9b38a 100644 --- a/in2lambda/draft/export.py +++ b/in2lambda/draft/export.py @@ -23,7 +23,7 @@ from in2lambda.api.set import Set from in2lambda.json_convert.json_convert import _question_stem, _question_title from in2lambda.source import DRAFT, ConversionToolsMissing, SourceError, frozen -from in2lambda.validation import _IMAGE, pdf +from in2lambda.validation import _IMAGE, _location, pdf _QUESTION = re.compile(r"q(\d+)\.text") """A question's text, and the number that orders it.""" @@ -119,6 +119,66 @@ def as_set(draft: dict[str, Any], directory: str = ".") -> Set: return question_set +def located(draft: dict[str, Any]) -> dict[str, str]: + """Which field of a draft each place `in2lambda.validation` reports against is. + + :func:`as_set` read backwards. The validator names a question, a part and a field + of the export, which is no address in the draft that wrote it, so this walks the + fields the way :func:`as_set` walks them and must be changed with it. + + Args: + draft: A draft, as `in2lambda.source.frozen` reads one. + + Returns: + The draft's field key for each location of the set it describes, the question's + own location included - where a problem about the whole question, such as an + image the export would not contain, is reported. A part answered by its + question's solution is located at that solution, since that is the field to go + and edit. Places no field of the draft wrote - a part's answer, the empty part + a question written without any exports as - are not here: nothing is in them + for the validator to find. + + Examples: + >>> from in2lambda.draft.export import located + >>> fields = {"q1.text": {"value": "State it."}, "q1.solution": {"value": "$x$"}} + >>> located({"fields": fields}) + {'Question 1 ""': 'q1.text', 'Question 1 "", main text': 'q1.text', 'Question 1 "", part (a), worked solution': 'q1.solution'} + >>> fields["q1.p1.text"] = {"value": "Do it."} + >>> fields["q1.p1.solution"] = {"value": ""} + >>> located({"fields": fields})['Question 1 "", part (a), worked solution'] + 'q1.solution' + """ + fields = draft["fields"] + where = {} + for number in sorted( + int(found[1]) for key in fields if (found := _QUESTION.fullmatch(key)) + ): + where[_location(number, "")] = f"q{number}.text" + where[_location(number, "", field="main text")] = f"q{number}.text" + parts = sorted( + int(found[2]) + for key in fields + if (found := _PART.fullmatch(key)) and int(found[1]) == number + ) + solution = f"q{number}.solution" + for index, part in enumerate(parts): + where[_location(number, "", index, "text")] = f"q{number}.p{part}.text" + written = f"q{number}.p{part}.solution" + # On the value and not the key, as `as_set` decides it: a solution field + # written empty leaves the part for its question's solution to answer. + if fields.get(written, {}).get("value"): + where[_location(number, "", index, "worked solution")] = written + elif solution in fields: + where[_location(number, "", index, "worked solution")] = solution + if solution in fields and all( + fields.get(f"q{number}.p{part}.solution", {}).get("value") for part in parts + ): + # The part `as_set` appends for a question's solution with no part left for + # it to answer, which is the last one and holds nothing else. + where[_location(number, "", len(parts), "worked solution")] = solution + return where + + def build(directory: str = ".", output_dir: str = "out") -> Path: """Writes the draft in a directory out as a Lambda Feedback set, if it is clean. diff --git a/in2lambda/draft/report.py b/in2lambda/draft/report.py index 0367d54..92ce094 100644 --- a/in2lambda/draft/report.py +++ b/in2lambda/draft/report.py @@ -8,21 +8,27 @@ to find out what is left to do, without reading the draft itself. Everything here reports, never refuses: what the checks found may well be deliberate, and -deciding that is whoever is writing the draft's to do. Only what is in the draft is -looked at - its blocks, its field keys, their ranges and their values - because what the -text of a question says is `in2lambda.validation`'s, at export. +deciding that is whoever is writing the draft's to do. The checks themselves read only +what is in the draft - its blocks, its field keys, their ranges and their values - and +what the text of a question says is `in2lambda.validation`'s: the set the draft describes +is exported and checked over as well, so that maths Lambda Feedback will not render is +reported against the field it is written in rather than found after uploading. """ import re +import warnings from pathlib import Path from typing import Any +from in2lambda.draft.export import as_set, located from in2lambda.source import DRAFT, frozen, save +from in2lambda.validation import pdf Finding = dict[str, Any] """One thing a check found: ``{"check", "field", "ranges", "message"}``. -``check`` is which check found it, ``field`` the block id or field key it is about, +``check`` is which check found it - ``problem`` where it was `in2lambda.validation`, +over the set the draft describes - ``field`` the block id or field key it is about, ``ranges`` the lines in question as ``[[start, end], ...]``, and ``message`` a sentence naming all of that, so that a line of the report can be acted on by itself. """ @@ -236,15 +242,90 @@ def checks(draft: dict[str, Any]) -> list[Finding]: + _without_solutions(draft) + _empty(draft) ) - return sorted( - found, - key=lambda finding: ( - finding["ranges"][0][0] if finding["ranges"] else _UNPLACED, - finding["field"], - ), + return sorted(found, key=_order) + + +def _order(finding: Finding) -> tuple[int | float, str]: + """Where a finding goes in a report: earliest line first, then by what it is about.""" + return ( + finding["ranges"][0][0] if finding["ranges"] else _UNPLACED, + finding["field"], ) +def problems(draft: dict[str, Any], directory: str = ".") -> list[Finding]: + """What `in2lambda.validation` finds in the set the draft describes. + + The draft is exported as it stands and the set checked over - maths delimiters, + what KaTeX will not render, images the export would not carry, and the compile + Lambda Feedback's PDF generator does - so that a question that will not render is + reported while the draft is being written rather than after it is uploaded. + + Args: + draft: A draft, as `in2lambda.source.frozen` reads one. + directory: Where the draft is, and so what the images it names are beside. + + Returns: + One :data:`Finding` per problem, named by the field of the draft it is in + rather than by the question and part of the export, so that a line of it can be + acted on with `field replace`. A problem about no one field - the set as a + whole failing to compile - keeps the validator's own naming of where it is. + + Warns: + UserWarning: pandoc or xelatex is not installed, so the set was not compiled. + """ + where = located(draft) + if not where: + # A draft with no question in it yet describes an empty set, which has nothing + # to find and is not worth a xelatex run to find it in. + return [] + + missing = pdf.missing_tools() + if missing: + # As `_katex_rejections` does without Node: a check that cannot be run here says + # what to install and leaves the rest of the report alone. + warnings.warn( + "The set the draft describes was not compiled as the PDF generator would: " + "install " + " and ".join(missing), + stacklevel=2, + ) + + fields = draft["fields"] + found = [] + for problem in as_set(draft, directory).problems(compile=not missing): + location = max( + (named for named in where if problem.location.startswith(named)), + key=len, + default="", + ) + if location: + key = where[location] + ranges = fields[key]["ranges"] + # Whatever the location says past the field: KaTeX names the characters of + # it that it stopped at, and those are the field's characters here as well. + rest = problem.location[len(location) :] + finding = { + "check": "problem", + "field": key, + "ranges": ranges, + "message": f"{key}{_where(ranges)}{rest}: {problem.message}", + } + else: + finding = { + "check": "problem", + "field": "", + "ranges": [], + "message": str(problem), + } + if finding not in found: + # A question's solution answers every part of it that has no solution of its + # own, so one fault in it is found once per part. They are the same field, + # the same lines and the same wording: a second line of the report saying so + # is a `field replace` that would be refused for finding nothing to replace. + found.append(finding) + return found + + def validate(directory: str = ".") -> list[Finding]: """Checks the draft in a directory over and writes the report into it. @@ -256,7 +337,8 @@ def validate(directory: str = ".") -> list[Finding]: directory: Where the ``draft.json`` to check is. Returns: - What the checks found, as it was written into the draft. + What the checks and `in2lambda.validation` found, as it was written into the + draft. Raises: DraftMissing: there is no draft in that directory. @@ -264,8 +346,11 @@ def validate(directory: str = ".") -> list[Finding]: SourceUnreadable: the markdown the draft names has moved, or is not text. DraftExists: the markdown has changed since the draft was written from it, so the lines the report named would not be the lines it was written about. + + Warns: + UserWarning: a check could not be run here - see :func:`problems`. """ draft, _ = frozen(directory) - draft["report"] = checks(draft) + draft["report"] = sorted(checks(draft) + problems(draft, directory), key=_order) save(Path(directory) / DRAFT, draft) return draft["report"] diff --git a/in2lambda/main.py b/in2lambda/main.py index 81319ba..35bb26f 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -424,8 +424,12 @@ def validate() -> None: Reports 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, parts nothing - answers, and fields holding nothing. Finding something is not a failure: the report - is written into draft.json either way, and replaced by the next one. + answers, and fields holding nothing. The set the draft describes is checked over as + well - 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 - each against the field it is written in. Finding something is not a + failure: the report is written into draft.json either way, and replaced by the next + one. """ with _message_not_traceback(): report = in2lambda.draft.report.validate() diff --git a/in2lambda/validation/__init__.py b/in2lambda/validation/__init__.py index 5a819d3..8b43cd6 100644 --- a/in2lambda/validation/__init__.py +++ b/in2lambda/validation/__init__.py @@ -59,6 +59,29 @@ class _Expression(NamedTuple): display: bool +def _location( + number: int, title: str, part: int | None = None, field: str | None = None +) -> str: + """Where in a set something is, as every message here names it. + + The one place that naming lives, since `in2lambda.draft.export` reads it backwards + to say which field of a draft a problem reported against it came from. + + Args: + number: The question's number, from 1. + title: The question's title, quoted even where it is empty. + part: Which part of the question, from 0, or None for the question itself. + field: Which field - ``main text``, ``worked solution`` - or None for the + question or the part as a whole. + """ + where = f'Question {number} "{title}"' + if part is not None: + where += f", part ({chr(ord('a') + part)})" + if field is not None: + where += f", {field}" + return where + + def validate(question_set: Set, compile: bool = True) -> list[Problem]: r"""Everything in2lambda can tell is wrong with a set, in the order it is written. @@ -99,8 +122,11 @@ def check( return _markdown_problems(markdown, question, location, expressions) for number, question in enumerate(question_set.questions, start=1): - where = f'Question {number} "{question.title}"' - problems += check(question.main_text, question, f"{where}, main text") + title = question.title + where = _location(number, title) + problems += check( + question.main_text, question, _location(number, title, field="main text") + ) images += question.images for image in question.images: @@ -108,13 +134,15 @@ def check( problems.append(Problem(where, f"there is no image file at {image}")) for index, part in enumerate(question.parts): - part_where = f"{where}, part ({chr(ord('a') + index)})" + part_where = _location(number, title, index) for field, markdown in ( ("text", part.text), ("worked solution", part.worked_solution), ("answer", part.answer), ): - problems += check(markdown, question, f"{part_where}, {field}") + problems += check( + markdown, question, _location(number, title, index, field) + ) for area_number, area in enumerate(part.response_areas, start=1): area_where = f"{part_where}, answer box {area_number}" diff --git a/tests/fixtures/drafts/README.md b/tests/fixtures/drafts/README.md index e94fbc4..82f2cbf 100644 --- a/tests/fixtures/drafts/README.md +++ b/tests/fixtures/drafts/README.md @@ -34,3 +34,8 @@ spaces for the question and eight for the parts, while its range still names the `question_without_parts` is a question and nothing else, which the checks have nothing to say about: it is here because a question with no parts is what the export has to write out as an empty part rather than as the template's. +`degrees` writes `^\circ` into the maths of both a question and the one worked solution answering +its two parts, and is the one folder whose report comes from `in2lambda.validation` over the set +the draft describes rather than from the checks over the draft itself: the solution is reported +against `q1.solution` rather than the parts it answers, since that is the field a `field replace` +would have to change, and reported once rather than once per part it was copied into. diff --git a/tests/fixtures/drafts/degrees/commands.json b/tests/fixtures/drafts/degrees/commands.json new file mode 100644 index 0000000..2423ab0 --- /dev/null +++ b/tests/fixtures/drafts/degrees/commands.json @@ -0,0 +1,40 @@ +[ + { + "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": "part add" + }, + { + "args": { + "question": "q1", + "text": "b5" + }, + "by": "tests", + "command": "question solution" + } +] diff --git a/tests/fixtures/drafts/degrees/expected.json b/tests/fixtures/drafts/degrees/expected.json new file mode 100644 index 0000000..9535887 --- /dev/null +++ b/tests/fixtures/drafts/degrees/expected.json @@ -0,0 +1,62 @@ +{ + "b1.ignore": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 1, + 1 + ] + ], + "value": true + }, + "q1.p1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 5, + 5 + ] + ], + "value": "Find the range." + }, + "q1.p2.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 7, + 7 + ] + ], + "value": "Find the time of flight." + }, + "q1.solution": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 9, + 9 + ] + ], + "value": "The range is $v^2 \\sin(2 \\times 52^{\\circ}) / g$, and the flight lasts twice that over the range times the launch speed." + }, + "q1.text": { + "by": "tests", + "edited": false, + "layer": 3, + "ranges": [ + [ + 3, + 3 + ] + ], + "value": "A projectile is launched at $52^{\\circ}$ to the horizontal." + } +} diff --git a/tests/fixtures/drafts/degrees/report.json b/tests/fixtures/drafts/degrees/report.json new file mode 100644 index 0000000..32b880f --- /dev/null +++ b/tests/fixtures/drafts/degrees/report.json @@ -0,0 +1,24 @@ +[ + { + "check": "problem", + "field": "q1.text", + "message": "q1.text (lines 3-3): ^\\circ does not display; write the degree sign \u00b0 instead", + "ranges": [ + [ + 3, + 3 + ] + ] + }, + { + "check": "problem", + "field": "q1.solution", + "message": "q1.solution (lines 9-9): ^\\circ does not display; write the degree sign \u00b0 instead", + "ranges": [ + [ + 9, + 9 + ] + ] + } +] diff --git a/tests/fixtures/drafts/degrees/source.md b/tests/fixtures/drafts/degrees/source.md new file mode 100644 index 0000000..1b77118 --- /dev/null +++ b/tests/fixtures/drafts/degrees/source.md @@ -0,0 +1,9 @@ +# Launch angle + +A projectile is launched at $52^{\circ}$ to the horizontal. + +Find the range. + +Find the time of flight. + +The range is $v^2 \sin(2 \times 52^{\circ}) / g$, and the flight lasts twice that over the range times the launch speed. diff --git a/tests/test_draft.py b/tests/test_draft.py index 27349dc..ed09b48 100644 --- a/tests/test_draft.py +++ b/tests/test_draft.py @@ -41,6 +41,9 @@ FIGURE = DRAFTS_DIR / "figure_in_a_question" """The one whose fields refer to an image file, which the export has to carry.""" +DEGREES = DRAFTS_DIR / "degrees" +"""The one whose report comes from the set the draft describes rather than the draft.""" + def _built(folder: Path, tmp_path: Path) -> Path: """A folder's document, frozen in `tmp_path` with its commands applied and checked.""" @@ -492,6 +495,95 @@ def test_a_clean_draft_is_reported_as_having_nothing_wrong_with_it( assert json.loads(draft_path.read_text())["report"] == [] +@needs_compiler +def test_validate_reports_what_the_pdf_generator_cannot_compile( + tmp_path: Path, monkeypatch +) -> None: + """The set a draft describes is compiled as well as read, against the field it is in. + + Only a compile says this: the fixtures cover what is found by reading the markdown, + and nothing there would tell a run with the toolchain installed from one without. + """ + monkeypatch.chdir(tmp_path) + _built(TWO_QUESTIONS, tmp_path) + replaced = CliRunner().invoke( + cli, + [ + "draft", + "field", + "replace", + "q1.solution", + "$Q = \\pi d^2 v / 4$", + "$x = \\nosuchcommand$", + ], + ) + assert replaced.exit_code == 0, replaced.output + + report = in2lambda.draft.report.validate() + + refused = [ + finding + for finding in report + if "the PDF generator cannot compile this" in finding["message"] + ] + assert [finding["field"] for finding in refused] == ["q1.solution"] + # Named as a field of the draft, since that is what `field replace` takes, rather + # than as the part of the export the solution ended up answering. + assert refused[0]["message"].startswith("q1.solution (lines 16-16): ") + + +@needs_compiler +def test_a_replay_without_the_toolchain_keeps_what_validate_found_with_it( + tmp_path: Path, monkeypatch +) -> None: + """A draft is replayed where it is read, which need not be where it was checked. + + Our own Docker image installs pandoc and no xelatex, so a report written here and + replayed there would come back a finding short if the replay ran the set checks + again - and the draft, untouched, would be called hand-edited. + """ + monkeypatch.chdir(tmp_path) + _built(TWO_QUESTIONS, tmp_path) + replaced = CliRunner().invoke( + cli, + [ + "draft", + "field", + "replace", + "q1.solution", + "$Q = \\pi d^2 v / 4$", + "$x = \\nosuchcommand$", + ], + ) + assert replaced.exit_code == 0, replaced.output + assert any( + "the PDF generator cannot compile this" in finding["message"] + for finding in in2lambda.draft.report.validate() + ) + + monkeypatch.setattr(pdf, "missing_tools", lambda: ["xelatex (how to install it)"]) + result = CliRunner().invoke(cli, ["draft", "replay"]) + + assert result.exit_code == 0, result.output + + +def test_validate_says_what_to_install_rather_than_reporting_the_compile( + tmp_path: Path, monkeypatch +) -> None: + """The toolchain is optional here as it is everywhere else: the rest still runs.""" + monkeypatch.chdir(tmp_path) + _built(DEGREES, tmp_path) + monkeypatch.setattr(pdf, "missing_tools", lambda: ["pandoc (how to install it)"]) + + with pytest.warns(UserWarning, match="pandoc"): + report = in2lambda.draft.report.validate() + + # What reading the markdown found, and nothing about the compile that was not run: + # the set is not compiled at all, so it has nothing to say about it either way. + assert [finding["field"] for finding in report] == ["q1.text", "q1.solution"] + assert not any("compile" in finding["message"] for finding in report) + + def test_a_command_run_after_a_report_leaves_none_behind( tmp_path: Path, monkeypatch ) -> None: