Skip to content

fix: treat i2's NotSet signature default as no default - #4

Merged
thorwhalen merged 1 commit into
masterfrom
not-set-defaults-are-required
Sep 22, 2026
Merged

thorwhalen merged 1 commit into
masterfrom
not-set-defaults-are-required

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

What

Step 2/3 of the re-land plan in i2mint/i2#48, for dagapp: i2's NotSet sentinel in a signature means "no default".

  • get_root_values: NotSet defaults are ignored, so the root gets the annotation's DFLT_VALS entry or 0.0, exactly as a param with no default does. Before, the sentinel itself became the widget's initial value.
  • _compute_node_value: a param missing from session state is only skipped (left to its default) if it has a real default (_has_default). A NotSet one behaves like a required param, as today.

No-op against the current i2: no current signature carries NotSet. There are no dependents in the fleet.

i2 compatibility (decision)

Uses i2.is_not_set (i2 >= 0.1.74, i2mint/i2#91) when available, otherwise falls back on i2.deco.NotSet (present since 2023). The i2 requirement is unchanged.

Tests

  • dagapp/tests/test_not_set_defaults.py: get_root_values for a DAG over a NotSet-defaulted function equals the plain one's, with no sentinel. Fails without the fix.
  • Doctest on _has_default.

Gate

  • pytest --doctest-modules dagapp (examples ignored), py3.12: 2 passed with i2 master and with i2 0.1.73 (fallback path). Master collects no tests.
  • ruff check / ruff format --check: clean.
  • Hosted CI: see checks.

🤖 Generated with Claude Code

`get_root_values` ignores NotSet defaults (falls back on the annotation's
default value or 0.0, as for a param with no default), and
`_compute_node_value` no longer skips a missing NotSet-defaulted param as
if it had a default. So a DAG built from signatures with NotSet defaults
(as a re-landed i2mint/i2#88 FuncFactory would show) never seeds a widget
with the sentinel. No-op for current signatures.

Uses `i2.is_not_set` when available, else falls back on `i2.deco.NotSet`.

Refs i2mint/i2#48

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@thorwhalen

Copy link
Copy Markdown
Member Author

Independent refute-review (sub-agent): dagapp itself is correct. The only places that read defaults are get_root_values and _compute_node_value. For normal params, _compute_node_value behaves as before (default None/0: skip; no default: KeyError; *args/**kwargs: unchanged). Both test runs pass (i2 master and i2 0.1.73). There is no test for the _compute_node_value change, which only differs for NotSet.

It also refuted my assumption that meshed needed no change. meshed sorted NotSet-defaulted params after required ones, which silently reorders a DAG's positional params. It also raised on merging a NotSet param with a same-named required one. That is fixed separately in i2mint/meshed#82. dagapp builds its roots from dag.sig, so it benefits from that fix, but this PR does not depend on it.

@thorwhalen
thorwhalen merged commit 2b6bcfa into master Sep 22, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the not-set-defaults-are-required branch September 22, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant