From 4a7395f2a95b4b2e9ef1f404115ed2ab8011e7ac Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Mon, 3 Aug 2026 15:49:37 -0500 Subject: [PATCH] Make ss config attribute assignment raise instead of silently no-op Assigning an attribute on a SuiteSparse config object used to write a plain instance attribute and leave the real config unchanged: gb.ss.config.nthreads = 1 config["nthreads"] stayed 18 (its real value), while attribute reads of gb.ss.config.nthreads then returned the dead 1 v.ss.config.sparsity_control = "bitmap" vanished entirely, since v.ss.config is built fresh per access BaseConfig now defines __setattr__ that raises AttributeError: - known writable option: points at item assignment, the supported write idiom (config["nthreads"] = value) - known read-only option: "is read-only", matching the ValueError text that item assignment gives - unknown name: "Unknown config option ...; known options are [...]". Item assignment of an unknown key already raises KeyError, so there is no advice that could silently create junk. The allowlist of internal attributes is a snapshot of the instance __dict__ taken when __init__ finishes, not a hardcoded list, so a subclass that adds instance attributes cannot silently fall out of sync. BaseConfig does not use __slots__, so the snapshot is the natural source of truth. Context.__init__ now sets gb_obj and _prev_context before calling super().__init__() so they exist by the time the snapshot is taken (Context._from_obj already set them before init). Properties such as Context._context pass through to their setters, so assigning the context itself keeps working and assigning a different context keeps raising from the property. About (gb.ss.about) had the same trap: about.mode = "junk" left about["mode"] alone but made attribute reads return "junk". It never sets instance attributes at all, so its new __setattr__ raises unconditionally: "is read-only" for known keys, "Unknown About option" otherwise. Item assignment, reads, iteration, repr, and IPython key completions are unchanged, as is the donfig-based gb.config guard. Pinned suite (suitesparse, blocking, no-mapnumpy): the suite gains exactly the 4 new tests, 145 skipped unchanged (each test, each verified to fail with the guard reverted). test_ss_utils.py still module-skips on the suitesparse-vanilla backend. --- graphblas/core/ss/config.py | 30 +++++++++++++++++ graphblas/core/ss/context.py | 6 ++-- graphblas/ss/_core.py | 7 ++++ graphblas/tests/test_ss_utils.py | 58 ++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/graphblas/core/ss/config.py b/graphblas/core/ss/config.py index e1ccd15a0..4d2a20573 100644 --- a/graphblas/core/ss/config.py +++ b/graphblas/core/ss/config.py @@ -52,8 +52,38 @@ def __init__(self, parent=None, context=None): if k not in rd: # pragma: no branch (safety) rd[k] = k cls._initialized = True + # Writes are unrestricted until __init__ finishes, and the instance + # attributes set by then become the allowlist for __setattr__. + # Subclasses that add instance attributes (such as Context) must set + # them before calling super().__init__(). + object.__setattr__(self, "_initializing", True) self._parent = parent self._context = context + object.__setattr__(self, "_internal_attrs", frozenset(self.__dict__) - {"_initializing"}) + object.__setattr__(self, "_initializing", False) + + def __setattr__(self, key, value): + # An instance built without running __init__ has no allowlist yet, so stay + # permissive rather than raising a confusing AttributeError from the guard. + # Properties (such as Context._context) go through so their setters run. + if ( + self.__dict__.get("_initializing", True) + or key in self._internal_attrs + or isinstance(getattr(type(self), key, None), property) + ): + object.__setattr__(self, key, value) + return + if (option := key.lower()) in self._options: + if option in self._read_only: + raise AttributeError(f"Config option {option!r} is read-only") + raise AttributeError( + f"Cannot set config option {option!r} by attribute assignment; " + f"this does not change the config. " + f"Use item assignment instead: config[{option!r}] = value" + ) + raise AttributeError( + f"Unknown config option {key!r}; known options are {sorted(self._options)}." + ) def __delitem__(self, key): raise TypeError("Configuration options can't be deleted.") diff --git a/graphblas/core/ss/context.py b/graphblas/core/ss/context.py index 67c7a6c20..99026e16f 100644 --- a/graphblas/core/ss/context.py +++ b/graphblas/core/ss/context.py @@ -33,9 +33,12 @@ class Context(BaseConfig): } def __init__(self, engage=True, *, stack=True, nthreads=None, chunk=None, gpu_id=None): - super().__init__() + # Instance attributes must exist before super().__init__() finishes, + # because BaseConfig.__setattr__ allows only the attributes set by then. self.gb_obj = ffi_new("GxB_Context*") check_status_carg(lib.GxB_Context_new(self.gb_obj), "Context", self.gb_obj[0]) + self._prev_context = None + super().__init__() if stack: context = threadlocal.context self["nthreads"] = context["nthreads"] if nthreads is None else nthreads @@ -49,7 +52,6 @@ def __init__(self, engage=True, *, stack=True, nthreads=None, chunk=None, gpu_id self["chunk"] = chunk if gpu_id is not None and "gpu_id" in self._options: self["gpu_id"] = gpu_id - self._prev_context = None if engage: self.engage() diff --git a/graphblas/ss/_core.py b/graphblas/ss/_core.py index 91e1496e9..1c005f294 100644 --- a/graphblas/ss/_core.py +++ b/graphblas/ss/_core.py @@ -316,6 +316,13 @@ def __getitem__(self, key): raise KeyError(key) raise _error_code_lookup[info](f"Failed to get info for {key}") # pragma: no cover (safety) + def __setattr__(self, key, value): + # About never sets instance attributes, so any attribute write is a + # mistake that would otherwise create a dead attribute shadowing reads. + if key.lower() in self: + raise AttributeError(f"About option {key.lower()!r} is read-only") + raise AttributeError(f"Unknown About option {key!r}; known options are {sorted(self)}.") + def __iter__(self): return iter( sorted( diff --git a/graphblas/tests/test_ss_utils.py b/graphblas/tests/test_ss_utils.py index 72d3952a4..30a664742 100644 --- a/graphblas/tests/test_ss_utils.py +++ b/graphblas/tests/test_ss_utils.py @@ -253,6 +253,64 @@ def test_global_config_key_completions(): about[key] +def test_global_config_attribute_assignment_raises(): + # Attribute assignment used to create a dead instance attribute and leave + # the real config unchanged; now it raises and points at item assignment. + config = gb.ss.config + before = config["nthreads"] + with pytest.raises(AttributeError, match="item assignment"): + config.nthreads = before + 1 + assert config["nthreads"] == before + assert "nthreads" not in config.__dict__ + with pytest.raises(AttributeError, match="Unknown config option 'nthread'"): + config.nthread = before + 1 + # Item assignment is the supported write and still works + config["nthreads"] = before + 1 + assert config["nthreads"] == before + 1 + config["nthreads"] = before + assert config["nthreads"] == before + # Reads, iteration, and key completions are unaffected by the guard + assert dict(config) == {k: config[k] for k in config} + assert set(config._ipython_key_completions_()) == set(config._options) + + +def test_object_config_attribute_assignment_raises(): + v = Vector(int, 3) + with pytest.raises(AttributeError, match="item assignment"): + v.ss.config.sparsity_control = "bitmap" + assert v.ss.config["sparsity_control"] == {"auto"} + with pytest.raises(AttributeError, match="read-only"): + v.ss.config.sparsity_status = "bitmap" + A = Matrix(int, 2, 2) + with pytest.raises(AttributeError, match="item assignment"): + A.ss.config.format = "by_col" + assert A.ss.config["format"] == "by_row" + # Item assignment is the supported write and still works + v.ss.config["sparsity_control"] = "bitmap" + assert v.ss.config["sparsity_control"] == {"bitmap"} + v.ss.config["sparsity_control"] = "auto" + assert v.ss.config["sparsity_control"] == {"auto"} + + +def test_about_attribute_assignment_raises(): + about = gb.ss.about + mode = about["mode"] + with pytest.raises(AttributeError, match="read-only"): + about.mode = "junk" + assert about["mode"] == mode + assert "mode" not in about.__dict__ + with pytest.raises(AttributeError, match="Unknown About option"): + about.junkattr = 1 + assert dict(about) == {k: about[k] for k in about} + + +@pytest.mark.skipif("gb.core.ss._IS_SSGB7") +def test_context_attribute_assignment_raises(): + context = gb.ss.Context(engage=False) + with pytest.raises(AttributeError, match="item assignment"): + context.nthreads = 4 + + @pytest.mark.skipif("gb.core.ss._IS_SSGB7") def test_context(): context = gb.ss.Context()