Skip to content

Make config attribute assignment raise instead of silently no-op - #608

Open
eriknw wants to merge 1 commit into
27-check-mask-movefrom
28-config-attr-guard
Open

Make config attribute assignment raise instead of silently no-op#608
eriknw wants to merge 1 commit into
27-check-mask-movefrom
28-config-attr-guard

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

Measured before this change: graphblas.config.autocompute = False
left config["autocompute"] at True and added a dead autocompute
entry to vars(config). donfig.Config defines no setattr for
options, so the write landed on the instance and the real option never
moved. The mistake is easy to make and leaves no trace.

graphblas.config is now an instance of a donfig.Config subclass whose
setattr rejects writes that are not donfig's own. For a known option
the AttributeError names the canonical idioms (config.set(name=value),
optionally as a with block, and config[name] to read). For an unknown
name it says so and lists the known options, deliberately WITHOUT
advising config.set(): donfig's set() accepts arbitrary keys, so pointing
there would trade a loud error for a silently-created bogus option, the
same class of trap this change exists to close.

The allowlist of writable attributes is derived rather than hardcoded:
the subclass snapshots vars(self) once donfig's init returns, so a
donfig release that adds or renames an internal field cannot break
attribute access here. Today that snapshot is name, env_prefix, env,
main_path, paths, defaults, deprecations, config, and config_lock. An
instance built without running init has no allowlist yet and stays
permissive, so the guard cannot raise from a half-built object.

What this does not change: reading options, config.set() as a call or
as a context manager, config[name], get(), update(), refresh(),
to_dict(), and writes to donfig's own attributes all behave as before.
Item assignment (config[name] = value) was never supported by donfig
and still raises TypeError.

Under the pinned config (--backend suitesparse --blocking
--no-mapnumpy), the suite goes from 1073 to 1075 passed by the two tests added here;
141 skipped is unchanged.


Stack created with GitHub Stacks CLIGive Feedback 💬

@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 28-config-attr-guard branch 3 times, most recently from b486d5c to 48219d6 Compare August 5, 2026 03:18
@eriknw
eriknw force-pushed the 28-config-attr-guard branch from 48219d6 to b06bc8f Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 28-config-attr-guard branch from b06bc8f to 6696c95 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 28-config-attr-guard branch 2 times, most recently from 58ab3df to b53eb4a Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 28-config-attr-guard branch from b53eb4a to 5109ed0 Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 28-config-attr-guard branch 3 times, most recently from ee5611f to f333736 Compare August 6, 2026 20:41
@eriknw
eriknw force-pushed the 28-config-attr-guard branch from f333736 to c73b194 Compare August 7, 2026 02:48
Measured before this change: `graphblas.config.autocompute = False`
left `config["autocompute"]` at True and added a dead `autocompute`
entry to `vars(config)`. donfig.Config defines no __setattr__ for
options, so the write landed on the instance and the real option never
moved. The mistake is easy to make and leaves no trace.

graphblas.config is now an instance of a donfig.Config subclass whose
__setattr__ rejects writes that are not donfig's own. For a known option
the AttributeError names the canonical idioms (config.set(name=value),
optionally as a `with` block, and config[name] to read). For an unknown
name it says so and lists the known options, deliberately WITHOUT
advising config.set(): donfig's set() accepts arbitrary keys, so pointing
there would trade a loud error for a silently-created bogus option, the
same class of trap this change exists to close.

The allowlist of writable attributes is derived rather than hardcoded:
the subclass snapshots vars(self) once donfig's __init__ returns, so a
donfig release that adds or renames an internal field cannot break
attribute access here. Today that snapshot is name, env_prefix, env,
main_path, paths, defaults, deprecations, config, and config_lock. An
instance built without running __init__ has no allowlist yet and stays
permissive, so the guard cannot raise from a half-built object.

What this does not change: reading options, config.set() as a call or
as a context manager, config[name], get(), update(), refresh(),
to_dict(), and writes to donfig's own attributes all behave as before.
Item assignment (config[name] = value) was never supported by donfig
and still raises TypeError.

Under the pinned config (--backend suitesparse --blocking
--no-mapnumpy), the suite goes from 1073 to 1075 passed by the two tests added here;
141 skipped is unchanged.
@eriknw
eriknw force-pushed the 28-config-attr-guard branch from c73b194 to dc4ba5d Compare August 7, 2026 05:09
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