Make config attribute assignment raise instead of silently no-op - #608
Open
eriknw wants to merge 1 commit into
Open
Make config attribute assignment raise instead of silently no-op#608eriknw wants to merge 1 commit into
eriknw wants to merge 1 commit into
Conversation
eriknw
marked this pull request as ready for review
August 4, 2026 16:07
eriknw
force-pushed
the
28-config-attr-guard
branch
3 times, most recently
from
August 5, 2026 03:18
b486d5c to
48219d6
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
from
August 5, 2026 17:44
48219d6 to
b06bc8f
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
from
August 5, 2026 18:03
b06bc8f to
6696c95
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
2 times, most recently
from
August 6, 2026 07:59
58ab3df to
b53eb4a
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
from
August 6, 2026 15:39
b53eb4a to
5109ed0
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
3 times, most recently
from
August 6, 2026 20:41
ee5611f to
f333736
Compare
eriknw
force-pushed
the
28-config-attr-guard
branch
from
August 7, 2026 02:48
f333736 to
c73b194
Compare
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
force-pushed
the
28-config-attr-guard
branch
from
August 7, 2026 05:09
c73b194 to
dc4ba5d
Compare
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
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.
Measured before this change:
graphblas.config.autocompute = Falseleft
config["autocompute"]at True and added a deadautocomputeentry to
vars(config). donfig.Config defines no setattr foroptions, 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
withblock, and config[name] to read). For an unknownname 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 CLI • Give Feedback 💬