Repository navigation
🐛 sphinx-needs: a TOML functions key warns instead of crashing - #2079
Merged
Merged
Conversation
A `functions` key under `[needs]` in the needs_from_toml file ends the build with `'str' object has no attribute '__name__'`, whatever its shape (a list, a string, or a table), because merge_default_configs registers every entry of needs_functions as a callable. The same crash comes from conf.py for an entry that is not callable. These tests pin the intended behaviour: the TOML key warns under needs.config and is dropped, a non-callable needs_functions entry warns and is skipped, and the build completes. Three guards pass already and must keep passing: an empty `functions = []` stays silent, a -D override of needs_functions wins silently, and a real function from conf.py is still registered beside a TOML file. The first four tests fail until the fix.
needs_functions holds Python callables, which a TOML file cannot hold, yet load_config_from_toml copied a `functions` key under `[needs]` into the configuration like any other, and merge_default_configs then crashed on the first entry with `'str' object has no attribute '__name__'`. A config field can now opt out of the TOML file through its metadata: `"toml": False`, with the reason for the warning as `"toml_reason"`, read through NeedsSphinxConfig.toml_ignored_reason. `_functions` is the one field that carries it. The loader checks it after the -D override check, so an override still wins silently, in the `[needs]` loop and, for symmetry, in the `[needs.schema]` loop; the key is reported as a needs.config warning and dropped, and the build continues. An empty value sets nothing, so it is dropped without a warning, as `functions = []` was silent before. The root cause is fixed too: merge_default_configs skips a needs_functions entry that is not callable with a needs.config warning instead of raising, which covers the same crash from conf.py. The configuration docs name the exception, and fix the `==` typo in the needs_functions example.
F1: a key that cannot be read from the TOML file has no silent form. The empty-value exception is gone: `functions = []`, `""` and an empty table are reported and ignored like any other value. The exception was not harmless, since an empty list used to replace, silently, the functions that conf.py registers; it now keeps them and warns. The flipped test pins the empty list, a new one pins conf.py's function surviving it, and the shape test also covers the empty string and the empty table. F2: the changelog no longer claims that a -D override wins without a warning (Sphinx splits a list override on commas, so it never could on the command line); the precedence test's docstring names confoverrides. F3: the changelog headline carries the changed-output marker. F4: the check in the `[needs.schema]` loop is removed: no key can reach it, and that loop is left to its own issue. F5: a field flagged without a `toml_reason` gets a generic reason instead of a KeyError. F6: the needs_functions example defines the function before the line that uses it, so it runs as written.
github-actions
Bot
added
the
pkg: sphinx-needs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2079 +/- ##
=======================================
Coverage 92.26% 92.26%
=======================================
Files 136 136
Lines 19062 19076 +14
=======================================
+ Hits 17587 17601 +14
Misses 1475 1475
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5 tasks done
chrisjsewell
added a commit
that referenced
this pull request
Oct 7, 2026
#2112) ## What Package: `packages/sphinx-needs`. Follow-up to #2079, which made a non-callable `needs_functions` entry a `needs.config` warning. Three neighbouring shapes still ended the build during `config-inited`, before a single document was read: | `conf.py` | on master | |---|---| | `needs_functions = None` | `TypeError: 'NoneType' object is not iterable` (from `merge_default_configs`) | | `needs_functions = my_function` | `TypeError: 'function' object is not iterable` | | `needs_functions = [functools.partial(f, x=1)]`, or an instance with `__call__` | `AttributeError: 'functools.partial' object has no attribute '__name__'` (from `add_function`) | A string, `needs_functions = "my_function"`, did not crash but was read one character at a time: one `needs_functions entry 'm' is not callable and is ignored` warning per character. Each is now reported and the build continues: - A value that is not a list (or tuple) is ONE warning naming its type, and nothing is registered from it (the built-in functions still are): ``` WARNING: needs_functions is of type 'NoneType', not a list of callables, and is ignored [needs.config] WARNING: needs_functions is of type 'function', not a list of callables, and is ignored [needs.config] WARNING: needs_functions is of type 'str', not a list of callables, and is ignored [needs.config] ``` Sphinx's own type check (`check_confval_types`, `config-inited` priority 800) would report the same value again (``The config value `needs_functions' has type `NoneType'; expected `list'.``, measured), so after warning the value is replaced with the confval's default `[]`; it is reported once, under `needs.config` like its neighbours. The declared `types=(list,)` is unchanged. A set of functions, or another iterable of functions that is neither a list nor a tuple (a frozenset, a generator), used to register its functions beside Sphinx's own type warning, and is now ignored with the same `needs.config` warning. A tuple is still registered, as before (Sphinx's own type warning for it is unchanged). - An entry that is callable but has no `__name__` is skipped with one warning, and the rest of the list is still registered: ``` WARNING: needs_functions entry functools.partial(<function echo_word at 0x...>, word='forty-two') has no __name__ and is ignored: an entry must be a callable with a __name__; use add_dynamic_function(app, func, name=...) for one without [needs.config] ``` - A non-callable entry keeps #2079's warning, unchanged. - The API path, `add_dynamic_function(app, func)` without `name=` for a callable without a `__name__`, raised a bare `AttributeError`. It now raises `NeedsApiConfigException` (as `add_warning` does for a missing argument): `Dynamic function functools.partial(...) has no __name__; pass the name to call it by with name=...`. The rule is one sentence in the `add_dynamic_function` docstring, and in the Registration section of `dynamic_functions.rst`. The changelog bullet is marked **(changed output)**: every changed shape crashed or already warned, but a set-valued `needs_functions` (`{my_function}`), which Sphinx already reported as the wrong type, had its functions registered and their `[[my_function()]]` calls resolved; it is now ignored, so those calls report `Unknown function 'my_function'`. ## Why Issue #2073, the follow-up of #2064's second item: `needs_functions` values and entries that end the build with a traceback instead of a configuration warning, so a mistake in `conf.py` is reported like the others and the build continues. Closes #2073 ## Tests All in `tests/test_needs_from_toml_functions.py`, the suite #2079 wrote for `needs_functions`; each build test is a real in-process build asserting the whole `build_warnings(app)` list and that the build completes. The tests for the crashing shapes were committed first and were red against master; the tuple test guards what master already did. | test | pins | |---|---| | `test_a_needs_functions_value_that_is_not_a_list_warns_once_and_is_ignored[none]` | `None`: one warning, no Sphinx duplicate, `needs_functions == []`, only the built-ins registered | | `…[bare-function]` | a bare function: the same | | `…[string]` | a string: one warning instead of one per character | | `…[set]` | a set: one warning, nothing registered from it (it used to register, beside Sphinx's type warning) | | `test_a_needs_functions_entry_without_a_name_warns_and_is_ignored[partial]` | a `functools.partial` is skipped with one warning; the next entry is registered and resolves in a need | | `…[instance]` | an instance with `__call__`: the same | | `test_a_needs_functions_tuple_is_still_registered` | a tuple still registers and resolves; the only warning is Sphinx's own ``has type `tuple'; expected `list'.`` | | `test_add_dynamic_function_asks_for_a_name_for_a_callable_without_one` | the API raises `NeedsApiConfigException` asking for `name=`, registers nothing; with `name=` it registers | Mutation checks (each applied, the file run, reverted): | mutation | red | |---|---| | remove the not-a-list guard | `[none]`, `[bare-function]` (crash), `[string]` (per-character list), `[set]` (registered beside Sphinx's warning) | | narrow the guard to `list` only | the tuple test (its function becomes `Unknown function 'answer'`) | | remove the no-`__name__` guard | `[partial]`, `[instance]` (crash) | | skip a nameless entry without warning | `[partial]`, `[instance]` (`assert [] == [...]`) | | `add_function` reads `function.__name__` again | the API test (`AttributeError`) | `uv run poe lint` and `uv run poe typecheck` pass. The full suite passes apart from the graphviz-rendering tests, which need `dot` and fail identically without this change on a machine that lacks it. ## Checklist - [x] I wrote this change myself and have read every line of it; it was not generated automatically from an issue. Built, adversarially reviewed and validated in an orchestrated Claude Code session from the issue, the code and the tests of #2079; every line was read by its builder, its reviewer and the orchestrator, and the tests were committed first and shown red against master. - [x] I ran the package's tests (`uv run poe test-needs`) and they pass (the graphviz renders excepted on a machine without `dot`, identically to master). - [x] Documentation is updated where behaviour or options change (`docs/dynamic_functions.rst`, the Registration section). - [x] The package's `docs/changelog.rst` has an entry under *Unreleased*. - [x] `uv run poe lint` and `uv run poe typecheck` pass. ## ubCode Nothing to match in ubCode, because ubCode registers no Python callables: `needs_functions` is a `conf.py` value it never reads (`docs/source/configuration/needs.rst:1684`: "Functions a project registers through ``needs_functions`` are not resolved"), and a `functions` key under `[needs]` in `ubproject.toml` is dropped silently, because `NeedsConfig` carries `#[serde(default)]` without `deny_unknown_fields` (`rust/ubc_config/src/needs/input.rs:23-25`); reporting unknown keys is the open useblocks/ubcode#2931. The shapes fixed here are Python-object shapes (a partial, `None`, a bare function) that cannot be written in TOML. (Measured at ubCode `origin/main`, `8bece77`.) ## Follow-ups - A tuple is accepted by sphinx-needs, but Sphinx's own check still warns that it is not a `list` (``The config value `needs_functions' has type `tuple'; expected `list'.``, unchanged from master). Declaring `types=(list, tuple)` would be a separate, deliberate change to the confval.
5 tasks done
chrisjsewell
added a commit
that referenced
this pull request
Oct 8, 2026
… order; cycles and out-of-scope reads warn (#2134) ## What Package: `packages/sphinx-needs`. Every `[[…]]`, `<<…>>` and `<{…}>` is now computed after every value it reads, instead of need by need in the order the needs reached the environment. Phase 1 of the derived-values contract shared with ubCode (its `planning/derived-need-values.md` §5.3): ```text needextend (as written) ─► stratum 1: link fields ─► back links ─► stratum 2: every other field ─► checks (user functions last) (need-id order) (user functions last) (link conditions, dead links, sort, constraints) ``` ```rst .. req:: A 🆔 REQ_A :status: [[copy("status", "REQ_B")]] .. req:: B 🆔 REQ_B :status: [[copy("status", "REQ_C")]] .. req:: C 🆔 REQ_C :status: done ``` On master `REQ_A` is `done` or `None` by which document Sphinx read first (and `needs.derive_unresolved` says so); now it is `done` in every build, serial or `-j`, scratch or incremental. - **`functions/order.py` (new)**: the nodes (`(need id, field)` for every field that carries a call after `needextend`), what each reads read off the call text (S/R1 edges for `copy`, one column node per field for `calc_sum` and a filter naming computed fields, the precomputed candidates of a filter on final values, the targets of the need's links in written order for `links_only` and `check_linked_values`, every condition's names for a variant), iterative Tarjan for the cycles, Kahn with a heap for the order (ties by need id, then field). Unit-tested per edge rule (a table over call texts) and on a 20 000-need chain. - **`resolve_functions`** (same signature) drives it: stratum 1, `build_backlinks`, stratum 2. The per-field body moved into `_resolve_field` unchanged but for the record's argument and the join (below). - **`resolve_links` splits**: `build_backlinks` (between the strata; each back link list in need-id order; the dead-link flags final there) and `check_links` (after stratum 2: link conditions read computed values and complete back links); `sort_links()` stays last. `resolve_links` remains, as the three in a row, for the benchmark. - **Cycles** (`needs.derive_cycle`): no member is computed; each holds its *placeholder* — a link or array field the items written in it (`LIT_1, [[copy("links")]]` keeps `LIT_1`, which keeps its back link), any other field (and a list with nothing written) its typed empty value (`None`, or `""`/`0`/`0.0`/`False`/`[]`). One warning per member field naming the members, saying which column the member itself reads the cycle through (a filter naming a computed field, a sum over every need, a filter on final values that keeps a need on the cycle) or that its variant reads the field it sets. `:hours: [[calc_sum("hours")]]` is one. - **Reads that cannot be ordered** (`needs.derive_scope`). A node one of whose calls reads what its stratum cannot wait for is a **sink**: it is not run, it reads nothing (so it is on no cycle), it holds its placeholder, and each such call is one warning. That is a link field's call or variant reading a computed field that is not a link field, a back link, or `has_dead_links`/`has_forbidden_dead_links` (set with the back links); and a `need.<field>` that selects what a call reads while it is computed in the same stratum. Also reported: a `needextend` filter naming a computed field (once per extend), and a built-in reading a field a user function computes. The last is found at run time by the read record phase 0 added, kept as the check that the static order covers every read a built-in makes: across the whole suite it fires only in the two tests built for that case, and never for a missed edge (whose message asks to be reported). A filter's reads are not seen by that check, so build tests pin each filter edge (below). - **Your own functions** run after the built-ins of their stratum, in need-id order; one in a link field (sphinx-test-reports' `tr_link`) runs at the end of stratum 1, so its links are in the back links. - **`need.<field>` arguments** are accepted in field and link values (the six parse sites), so a need using one is created instead of refused. One that selects what a built-in reads (an id, a field, a filter, `links_only`) must be final first, and fails the call (`needs.dynamic_function`) when its field is unset — `copy("title", need.src)` no longer copies the need's own title; one in a value argument (`result`, `search_value`, `upper`, …) is read like the field, after it is computed, and is `None` when unset; the `:ndf:` role in a need's content fails the same way (`??`). `copy(x, "ID", filter=…)` orders its `current_need[…]` reads on `ID`, which is the `current_need` it evaluates the filter with. - **A failed call** (it raises, or its result fails the type check) leaves its field at its placeholder, as a cycle member's: `:links: LIT_1, [[copy("hours")]]` keeps `LIT_1` (it was `[]`), with its `needs.dynamic_function` warning. - **A `None` result** adds nothing to a field: alone in a nullable string or array field it leaves it unset (it was stored as the text `"None"`, or `[None]` that schema validation refused). - A field a `needextend` sets to a call holds its placeholder until computed (the TODO in `extend_needs_data`'s REPLACE case): a replacing call the empty value, a call appended to written array items those items. - `needs.derive_unresolved` (#2080, never released) is retired; `ENV_DATA_VERSION` 9 → 10 (values change meaning: an upgraded build re-writes every page), pinned to the docstring paragraph that says so. - The back links are reset once per pass, at its start (so a function of your own in a link field reads none of an earlier in-process build), and `build_backlinks` builds into the empty lists at the barrier; the report of `needextend` filters naming a computed field walks the needs only when an extend has a filter. - The docs' own build adds `needs.derive_scope` to its `suppress_warnings`: its examples share one project, so the `needextend` examples filtering on `tags` or `status` name fields other pages' dynamic-function and variant examples compute (three reports), as the build already does for `needs.link_outgoing`. - Docs: `dynamic_functions.rst`' processing order rewritten for the strata, with **Cycles** and **Reads that cannot be ordered** replacing the `needs.derive_unresolved` section; the `need.<field>` argument form; on the needextend page, that a filter naming a computed field is `needs.derive_scope`, and that a `[[…]]` or `<<…>>` (which sees the needs after every `needextend`) is the way to react to what another `needextend` set. ## Why #2064 (the derived-values tracking issue; phase 0: #2078, #2079, #2080, #2081, #2083; the needextend flip: #2127). A computed value that read another one depended on the build history: document names, which documents an incremental build re-read, `-j`. Phase 0 named every such read; phase 1 orders them, so the value is a function of the sources, and turns what cannot be ordered into a finding instead of a silent placeholder. ## Tests The tests were committed first and shown red against the code before them (`68 failed` of the build tests, `ModuleNotFoundError` for the unit tests). After review, each behaviour change's tests were shown red against the reviewed head's sources (9 build tests), and the tests that pin behaviour already right (the filter edges the run-time check cannot see, a function named like a built-in, the `ENV_DATA_VERSION` pin, the appended-call hold, the empty list, the part's back links, the back-link reset) are each red under the deletion of what they pin. The reset is at the start of the pass: the rebuild test is red with it at the barrier only (`(['C1', 'C2'], ['C1', 'C2']) == ([], [])`), with no reset, and with a second reset at the barrier (the per-build count). | test | pins | |---|---| | `test_dynamic_functions_strata.py` (new, 46 items) | a chain in two layouts, serial and `-j 2`; a core field reading an extra one; extras in either declaration order; a sum over computed sums in two layouts; own links mixing a literal and a call (7.0); variants on a computed field, core and extra; cycles: a pair and a self-copy, a self-sum, a filter that makes every need a candidate, a self-reading variant, a call a needextend sets, a member an earlier in-process build had resolved; `copy("links_back")` in need-id order, `has_dead_links` final at the barrier; a link condition on `links_back` in both layouts; the `derive_scope` cases (link field reading a later value and a back link, a `need.<field>` selector computed in the same step, needextend filters, a built-in reading a user function's field); a `tr_link`-shaped user link function running last in stratum 1; `need.<field>` in field and link values; a `None` result. After review: a list field on a cycle keeps its written items, also after a `needextend` appended the call (`C_LINKS`, `X_ARR`, `X_ITEMS`; `X_REP`, a mixed string empty); the sinks — a link field reading another field (a literal kept, no dead link to `''`, no type error, no no-match arm), a back link, a later selector, and a blocked selector whose field another field reads (one `derive_scope`, no cycle); a link field reading `has_dead_links`; an unset `need.<field>` selector failing the call (also from a configured `default`) while an unset value is `None`; a `need.<field>` value chaining (`check_linked_values`' `search_value` and `result`, `copy`'s `upper`); each cycle member naming its own column; and, because the run-time check cannot see a filter's reads, one build per filter edge laid out so the reader sorts first: `copy`'s `current_need["k"]` (of the caller, and of the need named), `copy`'s and `check_linked_values`' filter on a computed field, `check_linked_values`' second target, `links_from_content`'s filter, and a user function registered under `copy`'s name; a field a `needextend` appended a call to, read before the call is computed (a string its empty value, not its written one; an array its written items); a call returning `[]` storing `[]`; a part's back links in need-id order at the barrier; an in-process rebuild dropping a removed back link; on both builds of an in-process rebuild, a function of your own in a link field reading no back link (its own need's or another's), with one reset per need per build; the `:ndf:` role failing on an unset `need.<field>` selector like the field does; a failed call, or a result its field cannot hold, leaving a link or array field its written items | | `test_env_data_version.py` (new, 1 item) | `ENV_DATA_VERSION` is the highest `Version N` its docstring names, so a bump without its paragraph, or a revert, is red | | `test_functions_order.py` (new, 78 items) | the edge table over call texts (a sink reads nothing: out-of-scope reads and blocked selectors carry no edge), the filter names, the typed empty values, cycles and each member's column, the step order, user functions last, the three causes of a run-time report, the 20 000-node chain | | `test_dynamic_functions_unresolved.py` (retargeted) | phase 0's 39 items on their projects with the ordered values: the message builder (now `derive_scope`'s), 17 value flips as phase-1 regressions (`test_chain_across_needs` stays THE layout test, `test_the_warning_does_not_depend_on_the_build_history` the incremental one), a failing reader of a cycle, the suppressions (and the retired type as a no-op), the subtype list; `test_ubcode_fixture` is the parity test on ubCode's fixture, `NEED_ATTR` back; `test_ubcode_own_field_copy_fixture` builds ubCode's `dynamic_functions_own_field_copy` pages verbatim (`BACKLINK`'s copy of no back link is `[]`) | | `test_dynamic_functions.py` | `TEST_7` created and resolved, `TEST_8` a `needs.dynamic_function`; the snapshot gains both | | `test_needextend_invalid_function.py` | the `need.<field>` cases now apply and resolve | | `test_variant_data_integration.py` | its six warnings in the order the values are computed (need id), not document order; its snapshot's `REQ_BADTYPE_ARRAY` keeps the written `a`, `c` beside the variant data value the array cannot hold | Mutations (each applied, the tests run, reverted): M1 stratum 2 in insertion order → `test_chain_across_needs[target-later]` red; M2 back links after stratum 2 → the `links_back` test red; M3 no precomputed candidates → `test_copy_filter_reads_the_match_it_copies_from` red (the false cycle); M4 no placeholder for cycle members → the in-process-rebuild cycle test and the written-items test red (with the needextend hold dropped too, the needextend-installed cycle test); a placeholder without the written items → the written-items and sink tests red; M5 a recursive Tarjan → the 20 000 chain red; M6 no variant-condition edges → the read record reports `derive_scope` and the variant test is red; M7 user functions in insertion order → the two sink tests red; M8 a sink still run → the `need.<field>` selector test and the sink test red (`V_LINK` `['LIT_2']`, `OTHER_RD` `['']`). Each filter edge the run-time check cannot see is red under its deletion: `copy`'s `current_need["k"]` read, `copy`'s and `check_linked_values`' filter-name reads, `check_linked_values`' targets after the first, `links_from_content`'s filter reads, and built-ins recognised by name instead of by the registered function. The full suite at the head (`uv run poe test-needs -n 4 tests/`): **50 failed, 2320 passed, 13 skipped** — the 50 are the graphviz renders (`tests/test_needflow.py`, the needflow conformance corpus, `test_doc_github_1664_legend[graphviz]`), which need `dot` and fail identically without this change on a machine that lacks it (the same 50 ids). 2320 = the base's 2191 (#2127's measured count; the base adds no sphinx-needs test to it) + 129 new items (46 + 78 + 1 in the three new files, two more in the retargeted file, two more in the needextend file). No test outside the files in the table changed value. The sibling suites that call into the resolver pass: `uv run poe test-reports` 157 passed, `uv run poe test-ub-test-reports` 356 passed (2 skipped); `uv run poe test-codelinks` 658 passed (3 skipped). `uv run poe docs-needs` builds with no warning of this change (the docs suppress the `needextend`-filter report their shared examples trigger, below); in this sandbox it still ends with 70 environmental warnings: the intersphinx hosts are refused by the proxy, and `dot` is absent. **Costs** (no sphinx-needs gate; quoted, not argued). Base `b4fe430` vs this head, `sphinx-build -b needs -E -q`, serial, every post-processing step timed (reviewer B's timer and driver), three rounds, base and head back to back per shape with the order alternating, paired per round, the 1-min load logged (1.1–5.2); synthetic projects of 10 needs per page, each `:links:` the previous need. The columns count the link-sorting loop on both sides (it moved out of `resolve_links` into `post_process_needs_data`): base = `resolve_functions` + `resolve_links`, head = `resolve_functions` (which now holds the graph and `build_backlinks`) + `check_links` + the sort loop. | shape · N | dynamic fields | base (median) | head (median) | paired delta (per round) | per dynamic field | of which the graph (`build_stratum`) | whole `post_process_needs_data`, paired | build wall base / head | |---|---|---|---|---|---|---|---|---| | copyall · 20 000 | 20 000 | 0.890 s | 1.597 s | +0.696, +1.144, +0.680 s | **+34.8 µs** (low-load round: +34.0) | 0.424 s = 21.2 µs | +1.903, +2.723, +1.851 s | 85.7 / 86.3 s | | chain10 · 20 000 | 18 000 | 2.214 s | 1.471 s | −0.946, −0.743, −0.528 s | **−41.3 µs** | 0.425 s = 23.6 µs | +0.111, +0.433, +0.870 s | 85.5 / 83.1 s | On `copyall · 20 000` (every need copies a field) phase 1 adds 0.70 s, 34.8 µs per dynamic field: 2.0× the recon's 17 µs upper estimate for the graph build and schedule. The graph itself is 21 µs (1.25×); the other ≈ 14 µs is the driver's bookkeeping per node (the `Project`, the pending set, what each node reads, a step each). Chain-heavy projects get faster (`chain10 · 20 000` −0.74 s: the phase-0 emission is gone), and `calcall · 2 000` is not slower (reviewer B's paired A/B runs: head faster in three of four pairs). The whole `post_process_needs_data` is +1.9 s on `copyall` and +0.4 s on `chain10` at 20 000 because it now carries a full garbage collection: every head run of both shapes has one gen-2 collection of 1.16–1.35 s in post-processing (inside `process_constraints`, 1.44–1.73 s against 0.25–0.31 s at base), and no base run has one there. It is not new: the build makes the same number of full collections in both versions (reviewer B measured 18, ≈ 8.6–9.0 s, over the whole build); at base the last one falls in the write phase, at head in `process_constraints`, so the build's wall time is unchanged within its ±7 s run-to-run spread. (An earlier version of this PR placed that collection inside the graph build, and an allocation fix moved it one step later; it did not remove it.) ## ubCode This is phase 1 of ubCode's `planning/derived-need-values.md` (§5.3): both tools compute derived values in the same strata and order, as one contract. Measured at this PR's head on ubCode's own phase-1 build fixtures (a frozen copy of ubCode's phase-1 change; each fixture converted to a Sphinx project reading its `ubproject.toml`, built with `-b needs`), per `(need, field)` that carries a call or a variant here: the value against the fixture's `__expected__/needs.json`, and the finding (`derive_cycle` / `derive_scope` / `dynamic_function`) against its `diagnostics.json`. ubCode's main is at `47ceb2f` (#3911, the type check); **its phase-1 pull request follows this one** (the number will be added here when it opens). **113 rows over the 14 fixtures; 110 agree** (value and finding): | fixture | rows | agree | the differing row | status | |---|---|---|---|---| | `dynamic_functions_chain` | 8 | 8 | — | | | `dynamic_functions_cycle` | 8 | 8 | — (7 `derive_cycle` in both, the same members) | | | `dynamic_functions_backlink` | 4 | 4 | — (`BAD_LINKS` `derive_scope` in both; its written link kept) | | | `dynamic_functions_removed_link` | 5 | 5 | — | | | `dynamic_functions_sum_column_resolved` | 3 | 3 | — | | | `dynamic_functions_unresolved` | 25 | 25 | — (`CYC_A`/`CYC_B` `derive_cycle`, `NEED_ATTR` `derive_scope` in both) | | | `dynamic_functions_unresolved_call_only` | 5 | 5 | — | | | `dynamic_functions_unresolved_page_added` | 1 | 1 | — | | | `dynamic_functions_unresolved_page_deleted` | 2 | 2 | — | | | `dynamic_functions_sum_column` | 10 | 10 | — | | | `dynamic_functions_cross_need` | 5 | 5 | — (`PROJECT_TOTAL` `dynamic_function` in both) | | | `dynamic_functions_own_field_copy` | 5 | 4 | `BACKLINK.incoming` (`copy('links_back')`, no need links to it): `[]` here, `null` in ubCode | ubCode moves: an empty back link list is `[]`; until ubCode's phase-1 change carries that, this row differs | | `dynamic_functions_type_check` | 21 | 20 | `F_BOOL_INT.line`: a boolean result in an integer field is stored here (Python's `bool` is an `int`; schema validation then reports it), unset + `needs.dynamic_function` in ubCode | registered (the boolean-in-integer row) | | `needs_variants` (with the fixture's `-t html -t draft`) | 11 | 10 | `REQ_006.status` (`<<[(((unclosed]: …>>`): unset + `needs.dynamic_function` here, the fallback arm + `needs.variant` in ubCode | registered (the unparsable-variant-condition row) | `cache_reproducibility` (no `__expected__`; built live in both tools, trimmed to what Sphinx builds): 20 rows, 16 agree, the 4 others the same unparsable-variant-condition row. An integer and an equal float count as the same value (the register's integer-in-number row). On review probes outside the fixtures (85 rows on my copies, 84 by reviewer A's counter; 80/79 agree) both tools now agree on: a link field reading another computed field, a back link or a computed array (not run, its written links kept), a variant in a link field reading a computed field, a blocked selector whose field another field reads (one `derive_scope`, no cycle), a link list on a cycle keeping its literal part, an array whose written items a `needextend` appended a call to (on a cycle: the written items), an unset `need.<field>` selector (a failed call), a `need.<field>` value argument (chained), the typed empty value of every type on a cycle, and a link list mixing a written link and a call that fails or returns a value the list cannot hold (`LL_NUM :links: LIT_1, [[copy("hours")]]`: `['LIT_1']` and `needs.dynamic_function` in both). The five rows that differ, by kind: | kind | rows | here | ubCode | |---|---|---|---| | registered in ubCode's divergence register | a link field's variant naming `has_dead_links` (`:links: NOPE, <<[has_dead_links]:LIT_1, LIT_2>>`) | not evaluated: `['NOPE']` + `needs.derive_scope` (the flag is final only with the back links) | the flag is not in its vocabulary: `needs.variant` "Unknown field", the no-match arm `['LIT_2', 'NOPE']` | | ubCode moves, in its own phase-1 pull request | `copy("links_back")` of an empty back link list, into an array field and into a string field (×2) | `[]`; into a string field the type-check finding | `null`, no finding | | ubCode moves, in its own phase-1 pull request | an authored array on a cycle mixing a written item and a call (`:items: a, [[copy("items", "Y")]]`); the same array whose call returns a value it cannot hold (`:items: a, [[copy("hours")]]`, measured outside the probe set) | keeps its written items, `['a']`, as a field a `needextend` appended a call to does | `null` (its authored placeholder is the typed empty value; its `needextend` placeholder already keeps the written items) | | not evaluated by ubCode | a `calc_sum` with a `filter` on a cycle (`FILT.amountn`) | `None` + `needs.derive_cycle` | `None`; ubCode does not evaluate the built-ins' `filter` arguments at all (its differences page says so), so this is no phase-1 row | The same three kinds are what the changelog names, with the fixtures' registered rows (a boolean in an integer field, an unparsable variant condition). The finding *texts* differ (each tool words its own); the members, needs and counts agree. A `None` result in a nullable field is now stored as `None`, not the text `"None"`, which closes the sphinx-needs half of the register's "None"-string row. ## Checklist - [x] I wrote this change myself and have read every line of it; it was not generated automatically from an issue. Built in an orchestrated session from #2064, ubCode's derived-values plan and the code; every test was run by hand, the tests were committed first and shown red, and every line was read. - [x] I ran the package's tests (`uv run poe test-needs`) and they pass (the graphviz renders excepted on a machine without `dot`, identically to master); `uv run poe test-reports` and `uv run poe test-ub-test-reports` pass. - [x] Documentation is updated where behaviour or options change (`docs/dynamic_functions.rst`, `docs/directives/needextend.rst`). - [x] The package's `docs/changelog.rst` has entries under *Unreleased* (#2080's and #2081's entries rewritten into the 9.0.0 statement, and a *Breaking changes* entry). - [x] `uv run poe lint` and `uv run poe typecheck` pass. The changelog: the #2080 and #2081 entries become ```rst - ✨ Dynamic functions and variants are computed in dependency order, and a value that cannot be computed is reported **(changed output)** (:issue:`2064`, :pr:`2080`, :pr:`2081`, :pr:`2134`) ``` and *Breaking changes* gains ```rst -‼️ Computed values are computed in dependency order **(changed output)** (:issue:`2064`, :pr:`2134`) ``` whose body lists what moves for a project (chained values, sums of computed summands, variants seeing computed fields, `links_back` readable, user functions after the built-ins, needs with `need.<field>` created) and that a cycle or a read that cannot be ordered is a new warning a `-W` build fails on — a cycle member, a link field whose call reads a later value and is not run, and a field whose call fails or returns a result the field cannot hold keep the items written in them if they are link or array fields and lose the computed ones, any other such field is left empty — with the `suppress_warnings` entries to keep it green meanwhile. The ✨ entry says of ubCode: ```rst `ubCode`_'s phase 1 computes the same values and reports the same findings, but for three kinds of difference. Registered in ubCode's divergence register: a boolean result in an integer field, an unparsable variant condition, and the dead-link flags ``has_dead_links`` and ``has_forbidden_dead_links``, which are not in ubCode's vocabulary (a condition naming one is reported there). Removed on ubCode's side by its own phase 1: a copy of an empty back link list (``[]`` here), and an authored array field that is not computed (on a cycle, or whose result the field cannot hold), which keeps its written items here. Not evaluated by ubCode at all: the ``filter`` arguments of the built-in functions. ``` Refs #2064.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
functionskey under[needs]in theneeds_from_tomlfile ends the build(exit 2), whatever its shape:
functions = ["x"],functions = "x"(iterated character by character) or a table such as
[needs.functions.linked_effort](its keys are iterated):
needs_functionsholds Python callables, which a TOML file cannot hold,but the loader copied the key into the configuration like any other.
Mechanism. A config field can now opt out of the TOML file through its metadata:
"toml": False, with the reason for the warning as"toml_reason"(a generic reason if it has none),read through a new
NeedsSphinxConfig.toml_ignored_reason(name)(which shares one field lookup with
convert_field_value)._functionsis the only field that carries the flag.load_config_from_tomlchecks it in the[needs]loop after the override check,so a
confoverridesvalue ofneeds_functionsstill wins and the TOML key is not reported.A flagged key is never read from the file: every occurrence is reported and ignored, and the build continues.
That includes an empty value (
functions = [],""or an empty table).An empty list used to replace the functions that
conf.pyregisters, without a warning.It now leaves them in place and is reported like any other value.
Message (
needs.config, so-Wstill fails on it andsuppress_warningscan silence it):It does not say that functions can never come from TOML.
The root cause in
conf.py. The same crash came fromconf.pyitself:needs_functions = ["x"]exits 2 with the same frames,because
merge_default_configstrusted every entry to be a callable.A non-callable entry is now skipped with
WARNING: needs_functions entry 'x' is not callable and is ignored [needs.config].The duplicate-function warning of
add_functionis unchanged.Changed output. Some builds that succeeded before now produce different output:
functions = []beside functions inconf.pyno longer removes them, and now warns;functions = ""and{}now give this warning instead of Sphinx's type warning;__name__(a module, say) inconf.py'sneeds_functionsnow warns.Docs.
configuration.rstnamesneeds_functionsas the exception to"the toml can contain any of the options below".
In the
needs_functionsexample it fixes the==typo and defines the function before the line that uses it.The changelog entry, marked (changed output), is under
Unreleased→Bug fixes.Part of #2064 (second item).
Tests
New
tests/test_needs_from_toml_functions.py, 10 tests:functionsas a list, a string, a table, an empty string and an empty table → exactly the warning above,needs_functions == [], status code 0.functions = []→ exactly the warning above.conf.py'sneeds_functions, plus a TOMLfunctions = []→the function still runs from
[[answer()]](forty-twoinneeds.json), and exactly the warning above.conf.pyneeds_functions = ["x"], no TOML → exactly the non-callable warning, status code 0.confoverrides={"needs_functions": []}with a TOMLfunctions = ["x"]→ no warning (the override wins and the key is not reported).conf.py'sneeds_functionsbeside a TOML file with other keys →the function runs (
needs.jsonhas its value), the TOML keys are read, no warning.Red first, against the base source (8 failed, 2 passed):
ExtensionErrorabove.The config value `needs_functions' has type `str'; expected `list'., ordict).answer:Unknown function 'answer' [needs.dynamic_function].Results:
uv run poe test-needs tests/test_needs_from_toml.py tests/test_variants_table.py tests/test_needs_from_toml_functions.py: 54 passed.uv run poe test-needs -n 4 -q -p no:cacheprovideron this branch (master3a9bcceplus these commits): 2094 passed, 13 skipped, 304 snapshots passed; no existing test or snapshot changed.uv run poe lintanduv run poe typecheck: exit 0.docs-needswas not run, because the sandbox's proxy blocks the intersphinx hosts.Follow-up
These are not in this PR:
[needs.services.foo]table in the TOML file with aclasskey crashes the build #2067:[needs.services.<name>]withclass = "x"andclass_init = {}in the TOML crashes the same way (AttributeError: 'str' object has no attribute 'options', inprepare_env).Warn when service classes are set in TOML #2068 proposes a fix with a
servicescheck at the same lines ofload_config_from_tomlthis PR touches,so whichever lands second needs a small conflict resolution; the per-field flag of this PR could carry that case too.
[needs.schema]table of the TOML file is never read: keys are skipped silently or crash the build #2072: the[needs.schema]loop ofload_config_from_toml.debug_activeis skipped as unknown.id_requiredcrashes withUnknown config field.needs_functionsentries that are callable without a__name__, or a non-iterable value, still crash the build #2073: theseneeds_functionsvalues still crash inadd_function:__name__, such as afunctools.partialor a callable instance;Noneor a bare function.When squashing, strip the
Co-authored-bytrailer GitHub proposes.