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()