From ee3ee7e39ba5c246f5ef1e4bd9b82cd1c516ca68 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 13:17:45 +0100 Subject: [PATCH 1/2] implement: Support Python predicates in specs (t27) --- docs/source/spec.md | 44 +++++- in2lambda/draft/__init__.py | 123 +++++++++++++--- in2lambda/main.py | 11 +- in2lambda/spec/__init__.py | 139 +++++++++++++++--- tests/fixtures/specs/README.md | 5 + tests/fixtures/specs/predicates/expected.json | 62 ++++++++ tests/fixtures/specs/predicates/predicates.py | 31 ++++ tests/fixtures/specs/predicates/source.md | 11 ++ tests/fixtures/specs/predicates/spec.yaml | 6 + tests/fixtures/specs/predicates/uncovered.txt | 1 + tests/test_spec.py | 136 +++++++++++++---- 11 files changed, 489 insertions(+), 80 deletions(-) create mode 100644 tests/fixtures/specs/predicates/expected.json create mode 100644 tests/fixtures/specs/predicates/predicates.py create mode 100644 tests/fixtures/specs/predicates/source.md create mode 100644 tests/fixtures/specs/predicates/spec.yaml create mode 100644 tests/fixtures/specs/predicates/uncovered.txt diff --git a/docs/source/spec.md b/docs/source/spec.md index 3a4f7bb..f0c934b 100644 --- a/docs/source/spec.md +++ b/docs/source/spec.md @@ -33,7 +33,8 @@ ignore: Header level=1 layout: PartsSepSol ``` -`question` and `layout` have to be there; `part`, `solution`, `strip` and `ignore` need not be. +`question` and `layout` have to be there; `part`, `solution`, `strip`, `ignore` and +`predicates` need not be. - **`question`, `part`, `solution`** select the blocks that are each of those things. - **`ignore`** selects the blocks that are none of them - a running header, a page of @@ -41,6 +42,8 @@ layout: PartsSepSol as left out. - **`strip`** is a list of patterns taken off the front of every value: the `(a) ` or `1. ` that labels a part in the document, but not in the question. +- **`predicates`** names a Python file beside the spec, for the selectors that cannot say what + they mean in constraints alone. See below. - **`layout`** is one of the [filters](filters/index), and says which solution answers which question or part. See below. @@ -77,6 +80,45 @@ the lines it came from. That is worth knowing in two places: a part written `(a) is a `ListItem`, because pandoc reads `(a)` as a list marker, and `strip` still has to take the `(a) ` off the front of the value, because the line it was copied from still has it. +## Predicates + +Some documents cannot be told apart by their text. If the questions are the paragraphs written +in bold, and a paragraph about marking starts with the word `Question` as surely as they do, +then no `text~` constraint will do it. For those, a spec names a Python file beside it and calls +functions from it: + +```yaml +predicates: predicates.py +question: Para bold_lead() +solution: Para italic_lead() +layout: PartsOneSol +``` + +A `name()` anywhere in a selector is a call, and goes with a type, with constraints and with +`after` - `after Header text=Solutions, is_solution()` - all of which have to hold as well. A +predicate is an ordinary function of one argument, the [panflute](https://scorreia.com/software/panflute/) +element the block is, that says whether the block is one of those: + +```python +import panflute as pf + + +def bold_lead(element: pf.Element) -> bool: + """Whether a block begins in bold.""" + first = element.content[0] if element.content else None + while isinstance(first, pf.Span) and first.content: # Past the sourcepos spans. + first = first.content[0] + return isinstance(first, pf.Strong) +``` + +The frozen source is parsed with pandoc's `sourcepos`, so that each block knows which lines it +came from, and that leaves every inline wrapped in a `Span` carrying where it is. A predicate +looking at the markup has to see through them, as the one above does. + +The file is named in the draft's log with its hash, exactly as the spec is, and it is run from +the bytes that hash was taken of. So a predicate edited after a run is refused the same way an +edited spec is, by `in2lambda draft replay` and by running the spec again. + ## Layouts The layout is the one thing that differs between problem sheets that are otherwise alike: where diff --git a/in2lambda/draft/__init__.py b/in2lambda/draft/__init__.py index cd88416..577963e 100644 --- a/in2lambda/draft/__init__.py +++ b/in2lambda/draft/__init__.py @@ -86,7 +86,10 @@ class ReplayDiffers(SourceError): class SpecChanged(SourceError): - """The spec file a log names is not the one that ran: it has changed, or gone.""" + """A file a log names - a spec, or its predicates - is not the one that ran. + + Either it has changed since, or it has gone. + """ def command(name: str) -> Callable[[Handler], Handler]: @@ -625,60 +628,134 @@ def _split_block( return f"{block}a and {block}b" -def _spec_as_run(directory: str, name: str, digest: str) -> bytes: - """A spec the log says has run, given it is still there and still says the same. +def _file_as_run(directory: str, name: str, digest: str) -> bytes: + """A file the log says a spec run used, given it still says what it said then. + + Args: + directory: Where the draft is, and so what the file is beside. + name: What the log calls the file: the spec, or the predicates it names. + digest: What the log says the file hashed to when it ran. + + Returns: + The contents of the file, for whoever is about to run it. Raises: SpecChanged: there is no such file beside the draft, or it is not the one the - log records running. Either way the fields it wrote are fields nothing on - disk would write again, so neither a replay nor another run can check them. + log records running. Either way the fields the spec wrote are fields nothing + on disk would write again, so neither a replay nor another run can check + them. """ try: raw = (Path(directory) / name).read_bytes() except FileNotFoundError: raise SpecChanged( f"There is no {name} beside {DRAFT}, and the log says the draft was filled " - "in from one. Put it back, or start the draft again with in2lambda source " + "in with it. Put it back, or start the draft again with in2lambda source " "add --start-over." ) from None if _digest(raw) != digest: raise SpecChanged( - f"{name} has changed since it was run against {DRAFT}, so the fields it " - "wrote are not the ones it would write now. Put it back, or start the " + f"{name} has changed since it was run against {DRAFT}, so the fields the " + "spec wrote are not the ones it would write now. Put it back, or start the " "draft again with in2lambda source add --start-over." ) return raw +def _files(args: dict[str, Any]) -> list[tuple[str, str]]: + """What a `spec run` entry says it ran, as ``(name, hash)`` for each file. + + The spec, and then the Python file of predicates it named, where it named one. + + Raises: + MalformedCommand: the entry names a file without hashing it, or the other way + about. + """ + files = [ + ( + _argument(args, "spec", "spec run"), + _argument(args, "hash", "spec run"), + ) + ] + if "predicates" in args or "predicates_hash" in args: + files.append( + ( + _argument(args, "predicates", "spec run"), + _argument(args, "predicates_hash", "spec run"), + ) + ) + return files + + +def spec_command(name: str, by: str, directory: str = ".") -> Command: + """The `spec run` entry for a spec, with everything it depends on hashed into it. + + The hashes go in the log beside the names, so that a replay can tell whether it is + running the files that wrote the fields it is checking. + + Args: + name: The spec to run, as it is to be named in the log: beside the draft. + by: Who is running it, as a name or a model. + directory: Where the draft is, and so what the spec is beside. + + Returns: + The command, for :func:`execute` to run. + + Raises: + SourceError: pandoc, panflute or pyyaml is missing; the spec cannot be read; or + it names a file of predicates that is not beside it. + """ + _require_conversion_tools() + raw = (Path(directory) / name).read_bytes() + args: dict[str, Any] = {"spec": name, "hash": _digest(raw)} + spec = in2lambda.spec.load(raw) + if spec.predicates is not None: + # Beside the spec, which is what a spec naming a file next to it means, and + # recorded from the draft's directory, which is what the log names things from. + beside = (Path(name).parent / spec.predicates).as_posix() + try: + code = (Path(directory) / beside).read_bytes() + except OSError: + raise in2lambda.spec.BadSpec( + f"There is no {beside} to read the spec's predicates from. The " + "functions a spec calls are in a Python file beside it." + ) from None + args |= {"predicates": beside, "predicates_hash": _digest(code)} + return {"command": "spec run", "args": args, "by": by} + + @command("spec run") def _spec_run( draft: dict[str, Any], markdown: str, args: dict[str, Any], by: str, directory: str ) -> str: """Fills in a draft's fields from a spec of selectors over the frozen source.""" _require_conversion_tools() - name = _argument(args, "spec", "spec run") - digest = _argument(args, "hash", "spec run") - # Every spec the log says has run, rather than one named the same way as this one: a - # spec that has been edited since leaves fields the log can no longer reproduce - # whatever it is spelled as now, so what has run is what to check. On a replay this - # re-reads specs whose own entries checked them, which is a file read each. + # Every file every spec the log says has run was run with, rather than one named the + # same way as this one: a spec that has been edited since leaves fields the log can + # no longer reproduce whatever it is spelled as now, so what has run is what to + # check. On a replay this re-reads files whose own entries checked them, which is a + # file read each. for entry in map(_checked, draft["log"]): if entry["command"] == "spec run": - _spec_as_run( - directory, - _argument(entry["args"], "spec", "spec run"), - _argument(entry["args"], "hash", "spec run"), - ) - raw = _spec_as_run(directory, name, digest) - - spec = in2lambda.spec.load(raw) + for file, digest in _files(entry["args"]): + _file_as_run(directory, file, digest) + raw = [_file_as_run(directory, file, digest) for file, digest in _files(args)] + + spec = in2lambda.spec.load(raw[0]) + functions = None + if spec.predicates is not None: + # Named by the entry rather than taken from the spec, so that what is run is the + # file the hash beside it in the log was checked against. + functions = in2lambda.spec.predicates( + spec, raw[-1], _argument(args, "predicates", "spec run") + ) # The blocks the selectors run over are the ones the parser makes of the source, and # a `split block` since has left the draft holding halves the parser never made. So # an ignored block is named and ranged from here rather than from the draft: the # field then spans the whole of what was ignored, and coverage, which goes by lines # as well as by name, counts each half of a split block as covered by it. elements = _elements(markdown) - fields, ignored = in2lambda.spec.fields(spec, elements, markdown) + fields, ignored = in2lambda.spec.fields(spec, elements, markdown, functions) for found in fields: record(draft, found.key, found.value, layer=1, ranges=found.ranges, by=by) lines = {block.id: [block.start, block.end] for block, _ in elements} diff --git a/in2lambda/main.py b/in2lambda/main.py index 0d163dd..edb2627 100644 --- a/in2lambda/main.py +++ b/in2lambda/main.py @@ -15,7 +15,6 @@ Iterator, ) from contextlib import contextmanager -from pathlib import Path from typing import Any, Optional import rich_click as click @@ -414,15 +413,7 @@ def spec_group() -> None: def spec_run(spec: str, by: str) -> None: """Fills the draft's fields in from SPEC, and says which blocks it left out.""" with _message_not_traceback(): - # The hash goes in the log beside the file's name, so that a replay can tell - # whether it is running the spec that wrote the fields it is checking. - report = in2lambda.draft.execute( - { - "command": "spec run", - "args": {"spec": spec, "hash": _digest(Path(spec).read_bytes())}, - "by": by, - } - ) + report = in2lambda.draft.execute(in2lambda.draft.spec_command(spec, by)) click.echo(report) diff --git a/in2lambda/spec/__init__.py b/in2lambda/spec/__init__.py index 10b503b..513eb64 100644 --- a/in2lambda/spec/__init__.py +++ b/in2lambda/spec/__init__.py @@ -10,6 +10,12 @@ ignore: Header level=1 layout: PartsSepSol +Where a selector cannot say it, a spec names a Python file beside it and calls functions +from it: ``predicates: predicates.py`` and then ``question: Para bold_lead()``, where +``bold_lead`` takes the panflute element and says whether the block is one. The file is +run by :func:`predicates` out of the bytes its caller hashed, so what runs is the file +the draft's log records having run. + Nothing here decides what a question is: the selectors say which blocks are which, and the layout says how a solution is paired up with the question or part it answers, which is the one thing that differs between the filters in :mod:`in2lambda.filters` and is @@ -21,13 +27,15 @@ """ import re +import types +from collections.abc import Callable # Rather than typing's, which beartype warns on. from dataclasses import dataclass, field from typing import Any, NamedTuple, Optional from in2lambda.filters import builtin_filters from in2lambda.source import Block, SourceError -_KEYS = ("question", "part", "solution", "strip", "ignore", "layout") +_KEYS = ("question", "part", "solution", "strip", "ignore", "layout", "predicates") """Everything a spec may say. Anything else in one is a typo, and is refused as one.""" _ATTRIBUTES = ("level", "text", "label") @@ -44,9 +52,14 @@ _TOKEN = re.compile( r"""\s*(?:(?P\w+)\s*(?P[=~])\s*""" - r"""(?:"(?P[^"]*)"|'(?P[^']*)'|(?P[^\s,]+))|(?P\w+))""" + r"""(?:"(?P[^"]*)"|'(?P[^']*)'|(?P[^\s,]+))""" + r"""|(?P\w+)\(\)|(?P\w+))""" ) -"""One word of a selector: a ``name=value`` or ``name~regex`` constraint, or a type.""" +"""One word of a selector: a constraint, a ``predicate()`` call, or a block type. + +The call comes before the type, so that ``bold_lead()`` is read as a call rather than as +a type named ``bold_lead`` with a pair of brackets nothing can make anything of. +""" class BadSpec(SourceError): @@ -80,31 +93,46 @@ class Selector: type: Optional[str] = None constraints: list[Constraint] = field(default_factory=list) + predicates: list[str] = field(default_factory=list) after: Optional["Selector"] = None - def matches(self, elements: list[Any], index: int, pf: Any) -> bool: + def matches( + self, + elements: list[Any], + index: int, + pf: Any, + functions: Optional[dict[str, Callable[[Any], Any]]] = None, + ) -> bool: """Whether the block at `index` is one of these. Args: elements: Every block of the document, as the panflute element it is. index: Which of them to decide about. pf: The panflute module, imported by the caller that has it. + functions: The predicates the spec's file holds, as :func:`predicates` bound + them, and None where the spec calls none. Returns: - True if the element is of this type, meets every constraint, and comes - after something the ``after`` selector matches. + True if the element is of this type, meets every constraint, satisfies every + predicate it calls, and comes after something the ``after`` selector matches. """ element = elements[index] if self.after is not None and not any( - self.after.matches(elements, earlier, pf) for earlier in range(index) + self.after.matches(elements, earlier, pf, functions) + for earlier in range(index) ): return False if self.type is not None and type(element).__name__ != self.type: return False - return all( + if not all( constraint.holds(_attribute(constraint.attribute, element, pf)) for constraint in self.constraints - ) + ): + return False + # Last, so that someone's own code only sees the blocks the rest of the selector + # has already agreed about - a predicate written for a Para is only given one. + called = functions or {} + return all(called[name](element) for name in self.predicates) @dataclass @@ -117,6 +145,8 @@ class Spec: solution: Optional[Selector] = None ignore: Optional[Selector] = None strip: "list[re.Pattern[str]]" = field(default_factory=list) + predicates: Optional[str] = None + """The Python file its selectors call functions from, where any of them do.""" class Field(NamedTuple): @@ -164,7 +194,9 @@ def _clause(text: str, line: int, after: Optional[Selector] = None) -> Selector: "name~'regex' constraints.", ) position = token.end() - if (name := token["name"]) is None: + if (called := token["predicate"]) is not None: + selector.predicates.append(called) + elif (name := token["name"]) is None: if selector.type is not None or selector.constraints: raise _refuse( line, @@ -247,8 +279,9 @@ def load(text: "str | bytes") -> Spec: Raises: BadSpec: the text is not YAML, is in an encoding YAML cannot read, is not a - mapping, says something a spec does not, or holds a selector, pattern or - layout that cannot be read. Every one of them says which line to look at. + mapping, says something a spec does not, holds a selector, pattern or layout + that cannot be read, or calls a function without naming the file its + functions are in. Every one of them says which line to look at. Examples: >>> from in2lambda.spec import load @@ -306,16 +339,81 @@ def load(text: "str | bytes") -> Spec: raise _refuse( lines["strip"], f"strip is a list of patterns, which {strip!r} is not." ) + + file = given.get("predicates") + if file is not None and not isinstance(file, str): + raise _refuse( + lines["predicates"], + f"predicates names a Python file beside the spec, which {file!r} is not.", + ) + question = _selector(given["question"], lines["question"]) + rest = { + role: _optional(given, role, lines) for role in _ROLES if role != "question" + } + if file is None: + for role, selector in {"question": question, **rest}.items(): + if selector is not None and (called := _called(selector)): + raise _refuse( + lines[role], + f"{called[0]}() is a function, and the spec does not say which " + "Python file its functions are in. Put the file beside the spec " + "and name it with a predicates: line.", + ) return Spec( - question=_selector(given["question"], lines["question"]), + question=question, layout=layout, - part=_optional(given, "part", lines), - solution=_optional(given, "solution", lines), - ignore=_optional(given, "ignore", lines), + part=rest["part"], + solution=rest["solution"], + ignore=rest["ignore"], strip=[_pattern(pattern, lines["strip"]) for pattern in strip], + predicates=file, ) +def _called(selector: Selector) -> list[str]: + """Every function a selector calls, its ``after`` clause included.""" + return selector.predicates + (_called(selector.after) if selector.after else []) + + +def predicates(spec: Spec, code: bytes, name: str) -> dict[str, Callable[[Any], Any]]: + """The functions a spec's selectors call, out of the file it names. + + Args: + spec: The spec whose selectors call them, as :func:`load` read it. + code: What the file holds, as the bytes its caller hashed. The file is run from + these rather than imported by its path, so that what runs is what was + checked against the hash the draft's log recorded. + name: What the file is called, for the traceback of anything it raises and for + the refusal of anything it has not got. + + Returns: + One callable per function the spec's selectors name, ready for + :meth:`Selector.matches`. + + Raises: + BadSpec: the file holds no function of a name a selector calls, or holds + something of that name that cannot be called. + """ + # Run as a module of its own rather than imported by path, so that nothing about + # where the file is - a name already imported, a stale .pyc - decides what runs. + module = types.ModuleType("in2lambda_predicates") + exec(compile(code, name, "exec"), module.__dict__) + found = {} + for role in _ROLES: + if (selector := getattr(spec, role)) is None: + continue + for called in _called(selector): + function = getattr(module, called, None) + if not callable(function): + raise BadSpec( + f"{name} has no function {called} in it, and the spec calls " + f"{called}(). A predicate is a function of one argument, the " + "panflute element, that says whether the block is one of those." + ) + found[called] = function + return found + + def _optional( given: dict[str, Any], name: str, lines: dict[str, int] ) -> Optional[Selector]: @@ -386,7 +484,10 @@ def _keys(layout: str, roles: list[Optional[str]]) -> list[Optional[str]]: def fields( - spec: Spec, elements: list[tuple[Block, Any]], markdown: str + spec: Spec, + elements: list[tuple[Block, Any]], + markdown: str, + functions: Optional[dict[str, Callable[[Any], Any]]] = None, ) -> tuple[list[Field], list[str]]: """What a spec makes of a document: its fields, and the blocks it says to ignore. @@ -395,6 +496,8 @@ def fields( elements: Every block of the frozen markdown beside the element it is, as :func:`in2lambda.source._elements` gives them. markdown: The frozen markdown itself, which the values are quoted out of. + functions: The predicates its selectors call, as :func:`predicates` bound them, + and None for a spec that calls none. Returns: One :class:`Field` per question, part and solution the spec found, and the ids @@ -410,7 +513,7 @@ def fields( role for role in _ROLES if (selector := getattr(spec, role)) is not None - and selector.matches(found, index, pf) + and selector.matches(found, index, pf, functions) ), None, ) diff --git a/tests/fixtures/specs/README.md b/tests/fixtures/specs/README.md index e3967e2..fdc3ffb 100644 --- a/tests/fixtures/specs/README.md +++ b/tests/fixtures/specs/README.md @@ -18,6 +18,11 @@ Each document is one the other three layouts read differently - a question with one solution, or with a part nobody answered - since a document every layout agrees about would pin no rule, and a layout given the wrong rule would go on passing. +`predicates` is the other thing a folder can pin: its spec names a `predicates.py` beside it and +calls functions from it, because what tells its questions from the paragraph about marks is the +bold each of them starts with, which is markup rather than text. Its log entry names that file +and hashes it as it does the spec, which is what makes a changed predicate refuse to replay. + A selector matches what pandoc parses, and a field holds the markdown of the lines it was taken from. That is why a part written `(a) Find the load.` is a `ListItem` - pandoc reads `(a)` as a list marker - while `strip` still has to take the `(a) ` off the front of the value. diff --git a/tests/fixtures/specs/predicates/expected.json b/tests/fixtures/specs/predicates/expected.json new file mode 100644 index 0000000..a005472 --- /dev/null +++ b/tests/fixtures/specs/predicates/expected.json @@ -0,0 +1,62 @@ +{ + "b1.ignore": { + "by": "tests", + "edited": false, + "layer": 1, + "ranges": [ + [ + 1, + 1 + ] + ], + "value": true + }, + "q1.solution": { + "by": "tests", + "edited": false, + "layer": 1, + "ranges": [ + [ + 7, + 7 + ] + ], + "value": "the load is $F = pA$, so the large piston carries 100 times the small one." + }, + "q1.text": { + "by": "tests", + "edited": false, + "layer": 1, + "ranges": [ + [ + 3, + 3 + ] + ], + "value": "A hydraulic scale has pistons of 20 mm and 200 mm diameter." + }, + "q2.solution": { + "by": "tests", + "edited": false, + "layer": 1, + "ranges": [ + [ + 11, + 11 + ] + ], + "value": "$p = \\rho g h$." + }, + "q2.text": { + "by": "tests", + "edited": false, + "layer": 1, + "ranges": [ + [ + 9, + 9 + ] + ], + "value": "Find the pressure 3 m below the surface of a lake." + } +} diff --git a/tests/fixtures/specs/predicates/predicates.py b/tests/fixtures/specs/predicates/predicates.py new file mode 100644 index 0000000..3078a14 --- /dev/null +++ b/tests/fixtures/specs/predicates/predicates.py @@ -0,0 +1,31 @@ +"""How the document beside this file writes a question and a solution: in its markup. + +A `text~` constraint cannot tell these apart, because the paragraph about marks begins +with the word Question as surely as the questions themselves do. What tells them apart +is the bold and the italics, which is markup rather than text, so it takes a predicate. +""" + +import panflute as pf + + +def bold_lead(element: pf.Element) -> bool: + """Whether a block begins in bold, which is how a question is written here.""" + return isinstance(_lead(element), pf.Strong) + + +def italic_lead(element: pf.Element) -> bool: + """Whether a block begins in italics, which is how a solution is written here.""" + return isinstance(_lead(element), pf.Emph) + + +def _lead(element: pf.Element) -> pf.Element: + """What a block starts with, past the spans the source is parsed wrapped in. + + A frozen source is parsed with sourcepos, so that a block knows which lines it came + from, and that leaves every inline inside a span carrying where it is. A predicate + looking at the markup has to see through them. + """ + first = element.content[0] if element.content else None + while isinstance(first, pf.Span) and first.content: + first = first.content[0] + return first diff --git a/tests/fixtures/specs/predicates/source.md b/tests/fixtures/specs/predicates/source.md new file mode 100644 index 0000000..43ce260 --- /dev/null +++ b/tests/fixtures/specs/predicates/source.md @@ -0,0 +1,11 @@ +# Hydrostatics problem sheet + +**Q1.** A hydraulic scale has pistons of 20 mm and 200 mm diameter. + +Question 1 was set in 2019 and is worth ten marks. + +*Solution:* the load is $F = pA$, so the large piston carries 100 times the small one. + +**Q2.** Find the pressure 3 m below the surface of a lake. + +*Solution:* $p = \rho g h$. diff --git a/tests/fixtures/specs/predicates/spec.yaml b/tests/fixtures/specs/predicates/spec.yaml new file mode 100644 index 0000000..243deb0 --- /dev/null +++ b/tests/fixtures/specs/predicates/spec.yaml @@ -0,0 +1,6 @@ +predicates: predicates.py +question: Para bold_lead() +solution: Para italic_lead() +strip: ['^\*\*Q\d+\.\*\* ', '^\*Solution:\* '] +ignore: Header +layout: PartsOneSol diff --git a/tests/fixtures/specs/predicates/uncovered.txt b/tests/fixtures/specs/predicates/uncovered.txt new file mode 100644 index 0000000..6d0875c --- /dev/null +++ b/tests/fixtures/specs/predicates/uncovered.txt @@ -0,0 +1 @@ +b3 diff --git a/tests/test_spec.py b/tests/test_spec.py index cc3699f..a0943cf 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -23,6 +23,12 @@ WORKED_EXAMPLE = SPECS_DIR / "parts_sep_sol" """The case the tests below happen to use; what they check holds for any of them.""" +PREDICATES = SPECS_DIR / "predicates" +"""The one whose spec calls functions from a file beside it, for the tests about those.""" + +HASHED = {"spec": "hash", "predicates": "predicates_hash"} +"""The files a `spec run` entry names, and what it calls each one's hash.""" + def _frozen(folder: Path, tmp_path: Path) -> CliRunner: """A folder's document and its spec, copied into `tmp_path` with the source frozen.""" @@ -32,6 +38,11 @@ def _frozen(folder: Path, tmp_path: Path) -> CliRunner: return runner +def _sha(path: Path) -> str: + """A file's hash, written the way the log writes one.""" + return f"sha256:{hashlib.sha256(path.read_bytes()).hexdigest()}" + + @pytest.mark.parametrize("folder", SPECS, ids=lambda path: path.name) def test_a_spec_fills_in_the_fields_beside_it_and_replays( folder: Path, tmp_path: Path, monkeypatch @@ -52,14 +63,18 @@ def test_a_spec_fills_in_the_fields_beside_it_and_replays( ] assert reported == (folder / "uncovered.txt").read_text().split() - # The spec is named and hashed in the log, so a replay runs the one that ran. - spec = (tmp_path / "spec.yaml").read_bytes() + # Every file the run needed is named and hashed in the log, so a replay runs the + # ones that ran - and a spec calling no functions names no file of them. + named = {"spec": "spec.yaml"} + if "predicates:" in (folder / "spec.yaml").read_text(): + named["predicates"] = "predicates.py" assert draft["log"] == [ { "command": "spec run", "args": { - "spec": "spec.yaml", - "hash": f"sha256:{hashlib.sha256(spec).hexdigest()}", + key: value + for file, name in named.items() + for key, value in ((file, name), (HASHED[file], _sha(tmp_path / name))) }, "by": "tests", } @@ -105,59 +120,75 @@ def test_a_spec_ignoring_a_block_the_draft_has_split_covers_both_halves( assert "is in no field" not in result.output +@pytest.mark.parametrize("file", ["spec.yaml", "predicates.py"]) def test_a_replay_is_refused_once_the_spec_has_changed( - tmp_path: Path, monkeypatch + file: str, tmp_path: Path, monkeypatch ) -> None: - """The fields came from the spec as it was, so a replay of a new one proves nothing.""" + """The fields came from the files as they were, so a replay of new ones proves nothing.""" monkeypatch.setenv("COLUMNS", "200") # So the message is not wrapped mid-sentence. monkeypatch.chdir(tmp_path) - runner = _frozen(WORKED_EXAMPLE, tmp_path) + runner = _frozen(PREDICATES, tmp_path) assert runner.invoke(cli, ["spec", "run", "spec.yaml"]).exit_code == 0 draft_path = tmp_path / "draft.json" written = draft_path.read_bytes() - spec = tmp_path / "spec.yaml" - spec.write_text(spec.read_text().replace("PartsSepSol", "PartsOneSol")) + _edit(tmp_path / file) result = runner.invoke(cli, ["draft", "replay"]) assert result.exit_code != 0 - assert "spec.yaml has changed" in result.output + assert f"{file} has changed" in result.output assert draft_path.read_bytes() == written +def _edit(path: Path) -> None: + """Changes a file without changing what it says, which YAML and Python both allow.""" + path.write_bytes(path.read_bytes() + b"\n# Changed since it was run.\n") + + @pytest.mark.parametrize( - "named", - ["spec.yaml", "./spec.yaml", "spec2.yaml"], - ids=["as it was", "spelled another way", "as a copy"], + ("file", "named"), + [ + ("spec.yaml", "spec.yaml"), + ("spec.yaml", "./spec.yaml"), + ("spec.yaml", "spec2.yaml"), + ("predicates.py", "spec.yaml"), + ], + ids=[ + "as it was", + "spelled another way", + "as a copy", + "its predicates edited", + ], ) def test_running_an_edited_spec_again_is_refused( - named: str, tmp_path: Path, monkeypatch + file: str, named: str, tmp_path: Path, monkeypatch ) -> None: """The fields of the first run would stay, and the draft could never replay again.""" monkeypatch.setenv("COLUMNS", "200") monkeypatch.chdir(tmp_path) - runner = _frozen(WORKED_EXAMPLE, tmp_path) + runner = _frozen(PREDICATES, tmp_path) assert runner.invoke(cli, ["spec", "run", "spec.yaml"]).exit_code == 0 draft_path = tmp_path / "draft.json" written = draft_path.read_bytes() - spec = tmp_path / "spec.yaml" - spec.write_text(spec.read_text().replace("PartsSepSol", "PartsOneSol")) + edited = tmp_path / file + was = edited.read_bytes() + _edit(edited) # However the second run names the spec - the way the first did, another way round # to the same file, or as a copy under a name of its own - what is refused is that - # the spec the draft was filled in from has changed, since that is what no replay + # a file the draft was filled in from has changed, since that is what no replay # could get past afterwards. - shutil.copy(spec, tmp_path / "spec2.yaml") + shutil.copy(tmp_path / "spec.yaml", tmp_path / "spec2.yaml") result = runner.invoke(cli, ["spec", "run", named]) assert result.exit_code != 0 - # Named as the spec that ran, whatever this run called it, and refused for having - # changed rather than for the fields of the first run being in the way. - assert "spec.yaml has changed" in result.output + # Named as the file that ran, whatever this run called the spec, and refused for + # having changed rather than for the fields of the first run being in the way. + assert f"{file} has changed" in result.output assert "--start-over" in result.output assert draft_path.read_bytes() == written # And what is on disk is still a draft that replays, which is the point of refusing. - spec.write_text(spec.read_text().replace("PartsOneSol", "PartsSepSol")) + edited.write_bytes(was) assert runner.invoke(cli, ["draft", "replay"]).exit_code == 0 @@ -186,20 +217,28 @@ def test_a_spec_run_over_a_log_holding_something_that_is_not_a_command_is_refuse assert draft_path.read_bytes() == written +@pytest.mark.parametrize("file", ["spec.yaml", "predicates.py"]) def test_a_replay_is_refused_once_the_spec_has_gone( - tmp_path: Path, monkeypatch + file: str, tmp_path: Path, monkeypatch ) -> None: - """A draft names the spec that filled it in, and someone may well have moved it.""" + """A draft names the files that filled it in, and someone may well have moved one.""" monkeypatch.setenv("COLUMNS", "200") monkeypatch.chdir(tmp_path) - runner = _frozen(WORKED_EXAMPLE, tmp_path) + runner = _frozen(PREDICATES, tmp_path) assert runner.invoke(cli, ["spec", "run", "spec.yaml"]).exit_code == 0 - (tmp_path / "spec.yaml").unlink() + written = (tmp_path / "draft.json").read_bytes() + (tmp_path / file).unlink() result = runner.invoke(cli, ["draft", "replay"]) + again = runner.invoke(cli, ["spec", "run", "spec.yaml"]) + # Neither a replay nor another run can check fields written by a file that is not + # there to write them again. assert result.exit_code != 0 - assert "spec.yaml" in result.output + assert file in result.output + assert again.exit_code != 0 + assert file in again.output + assert (tmp_path / "draft.json").read_bytes() == written @pytest.mark.parametrize( @@ -212,6 +251,12 @@ def test_a_replay_is_refused_once_the_spec_has_gone( ("question: Sausage\nlayout: PartsOneSol\n", "line 1", "pandoc element"), ("question: Header colour=blue\nlayout: PartsOneSol\n", "line 1", "colour"), ("quesiton: Header\nlayout: PartsOneSol\n", "line 1", "quesiton"), + ("question: Para lead()\nlayout: PartsOneSol\n", "line 1", "predicates:"), + ( + "question: Header\npredicates: 5\nlayout: PartsOneSol\n", + "line 2", + "predicates names a Python file", + ), ], ids=[ "not yaml", @@ -221,6 +266,8 @@ def test_a_replay_is_refused_once_the_spec_has_gone( "unknown type", "unknown attribute", "typo", + "a function with no file to find it in", + "predicates that is not a file name", ], ) def test_a_spec_that_cannot_be_read_says_which_line_to_look_at( @@ -243,6 +290,39 @@ def test_a_spec_that_cannot_be_read_says_which_line_to_look_at( assert (tmp_path / "draft.json").read_bytes() == written +@pytest.mark.parametrize( + ("spec", "named"), + [ + ( + "predicates: nowhere.py\nquestion: Para bold_lead()\nlayout: PartsOneSol\n", + ["nowhere.py"], + ), + ( + "predicates: predicates.py\nquestion: Para sausage()\nlayout: PartsOneSol\n", + ["predicates.py", "sausage"], + ), + ], + ids=["a file that is not there", "a function the file has not got"], +) +def test_a_spec_calling_a_predicate_nothing_holds_is_refused( + spec: str, named: list[str], tmp_path: Path, monkeypatch +) -> None: + """A predicate is named twice - in the spec and in the file - so it can be misspelt.""" + monkeypatch.setenv("COLUMNS", "200") + monkeypatch.chdir(tmp_path) + runner = _frozen(PREDICATES, tmp_path) + written = (tmp_path / "draft.json").read_bytes() + (tmp_path / "spec.yaml").write_text(spec) + + result = runner.invoke(cli, ["spec", "run", "spec.yaml"]) + + assert result.exit_code != 0 + for name in named: + assert name in result.output + assert isinstance(result.exception, SystemExit) + assert (tmp_path / "draft.json").read_bytes() == written + + def test_a_spec_saved_as_utf_16_is_read_like_any_other( tmp_path: Path, monkeypatch ) -> None: From 0486df9de4998b73efcc212a57f11baafe1273e5 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Sun, 20 Sep 2026 13:28:45 +0100 Subject: [PATCH 2/2] implement: Support Python predicates in specs (t27) --- in2lambda/draft/__init__.py | 13 ++++++++++--- in2lambda/spec/__init__.py | 14 ++++++++++---- tests/test_spec.py | 17 +++++++++++++++++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/in2lambda/draft/__init__.py b/in2lambda/draft/__init__.py index 577963e..04e1b4a 100644 --- a/in2lambda/draft/__init__.py +++ b/in2lambda/draft/__init__.py @@ -706,12 +706,19 @@ def spec_command(name: str, by: str, directory: str = ".") -> Command: it names a file of predicates that is not beside it. """ _require_conversion_tools() - raw = (Path(directory) / name).read_bytes() + try: + raw = (Path(directory) / name).read_bytes() + except OSError: + raise in2lambda.spec.BadSpec( + f"There is no {name} to read a spec from. A spec is the file of selectors " + "the draft's fields are filled in from." + ) from None args: dict[str, Any] = {"spec": name, "hash": _digest(raw)} spec = in2lambda.spec.load(raw) if spec.predicates is not None: - # Beside the spec, which is what a spec naming a file next to it means, and - # recorded from the draft's directory, which is what the log names things from. + # Beside the spec, which is what a spec naming a file next to it means and all + # that load lets one name, and recorded from the draft's directory, which is + # what the log names things from. beside = (Path(name).parent / spec.predicates).as_posix() try: code = (Path(directory) / beside).read_bytes() diff --git a/in2lambda/spec/__init__.py b/in2lambda/spec/__init__.py index 513eb64..0dfafdc 100644 --- a/in2lambda/spec/__init__.py +++ b/in2lambda/spec/__init__.py @@ -30,6 +30,7 @@ import types from collections.abc import Callable # Rather than typing's, which beartype warns on. from dataclasses import dataclass, field +from pathlib import Path from typing import Any, NamedTuple, Optional from in2lambda.filters import builtin_filters @@ -280,8 +281,9 @@ def load(text: "str | bytes") -> Spec: Raises: BadSpec: the text is not YAML, is in an encoding YAML cannot read, is not a mapping, says something a spec does not, holds a selector, pattern or layout - that cannot be read, or calls a function without naming the file its - functions are in. Every one of them says which line to look at. + that cannot be read, names a file of predicates that is not beside it, or + calls a function without naming the file its functions are in. Every one of + them says which line to look at. Examples: >>> from in2lambda.spec import load @@ -341,10 +343,14 @@ def load(text: "str | bytes") -> Spec: ) file = given.get("predicates") - if file is not None and not isinstance(file, str): + # Beside the spec, and so a name with nothing of a path in it. A spec that could + # name a file anywhere would run and log one the folder it is in does not hold, and + # the draft would then only replay where that file still sat outside the folder. + if file is not None and (not isinstance(file, str) or Path(file).name != file): raise _refuse( lines["predicates"], - f"predicates names a Python file beside the spec, which {file!r} is not.", + f"predicates names a Python file beside the spec, which {file!r} is not. " + "The name has no directory in it: the file is in the spec's own folder.", ) question = _selector(given["question"], lines["question"]) rest = { diff --git a/tests/test_spec.py b/tests/test_spec.py index a0943cf..a113928 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -18,7 +18,9 @@ from click.testing import CliRunner from conftest import SPECS, SPECS_DIR +import in2lambda.draft from in2lambda.main import cli +from in2lambda.source import SourceError WORKED_EXAMPLE = SPECS_DIR / "parts_sep_sol" """The case the tests below happen to use; what they check holds for any of them.""" @@ -257,6 +259,11 @@ def test_a_replay_is_refused_once_the_spec_has_gone( "line 2", "predicates names a Python file", ), + ( + "question: Para lead()\npredicates: ../shared.py\nlayout: PartsOneSol\n", + "line 2", + "../shared.py", + ), ], ids=[ "not yaml", @@ -268,6 +275,7 @@ def test_a_replay_is_refused_once_the_spec_has_gone( "typo", "a function with no file to find it in", "predicates that is not a file name", + "predicates somewhere other than beside the spec", ], ) def test_a_spec_that_cannot_be_read_says_which_line_to_look_at( @@ -323,6 +331,15 @@ def test_a_spec_calling_a_predicate_nothing_holds_is_refused( assert (tmp_path / "draft.json").read_bytes() == written +def test_a_spec_that_is_not_there_is_refused_rather_than_left_to_the_file_system( + tmp_path: Path, monkeypatch +) -> None: + """The command line checks the path; a script calling this straight does not.""" + monkeypatch.chdir(tmp_path) + with pytest.raises(SourceError, match="nowhere.yaml"): + in2lambda.draft.spec_command("nowhere.yaml", "tests") + + def test_a_spec_saved_as_utf_16_is_read_like_any_other( tmp_path: Path, monkeypatch ) -> None: