diff --git a/graphblas/__init__.py b/graphblas/__init__.py index d6a2d6e72..0868a45c8 100644 --- a/graphblas/__init__.py +++ b/graphblas/__init__.py @@ -25,7 +25,45 @@ def get_config(): import donfig import yaml - config = donfig.Config("graphblas") + class Config(donfig.Config): + """donfig Config that rejects silent attribute writes. + + Options are set with ``config.set(name=value)`` (optionally as a + ``with config.set(name=value):`` block) and read with ``config[name]``. + Plain attribute assignment (``config.name = value``) would otherwise + create a dead instance attribute and leave the real option unchanged, + so we raise instead. + """ + + def __init__(self, *args, **kwargs): + # Writes are unrestricted until donfig's own __init__ finishes, and + # the attributes it set there become the allowlist. Deriving that + # list beats hardcoding donfig's internals: a donfig release that + # adds or renames a field can't then break attribute access here. + object.__setattr__(self, "_initializing", True) + super().__init__(*args, **kwargs) + object.__setattr__(self, "_donfig_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. + if self.__dict__.get("_initializing", True) or key in self._donfig_attrs: + object.__setattr__(self, key, value) + return + if key in self.config: + raise AttributeError( + f"Cannot set config option {key!r} by attribute assignment. " + f"Use `graphblas.config.set({key}=...)` to change it (optionally " + f"in a `with` block for a scoped change) and " + f"`graphblas.config[{key!r}]` to read it." + ) + raise AttributeError( + f"Unknown config option {key!r}; known options are {sorted(self.config)}." + ) + + config = Config("graphblas") path = Path(__file__).parent / "graphblas.yaml" with path.open() as f: defaults = yaml.safe_load(f) diff --git a/graphblas/tests/test_core.py b/graphblas/tests/test_core.py index a15da6f54..859c5c21c 100644 --- a/graphblas/tests/test_core.py +++ b/graphblas/tests/test_core.py @@ -94,3 +94,48 @@ def test_packages(): def test_index_max(): assert gb.MAX_SIZE == 2**60 # True for all current backends + + +def test_config_attribute_assignment_raises(): + # Setting an option by plain attribute assignment used to silently no-op: + # donfig has no __setattr__ for options, so it created a dead instance + # attribute and left the real option alone. It should raise and name the + # canonical API instead. + orig = gb.config["autocompute"] + try: + with pytest.raises(AttributeError, match=r"config\.set\(autocompute"): + gb.config.autocompute = not orig + assert gb.config["autocompute"] == orig # the failed write changed nothing + # An unknown name is a mistaken write too, not a new attribute + with pytest.raises(AttributeError, match="not_a_real_option"): + gb.config.not_a_real_option = 5 + assert "not_a_real_option" not in vars(gb.config) + # Canonical idioms still work: read by item, write by set() + gb.config.set(autocompute=not orig) + assert gb.config["autocompute"] == (not orig) + with gb.config.set(autocompute=orig): + assert gb.config["autocompute"] == orig + assert gb.config["autocompute"] == (not orig) # restored on exit + finally: + gb.config.set(autocompute=orig) + + +def test_config_donfig_attrs_are_derived(monkeypatch): + # The allowlist of writable attributes must come from what donfig's + # __init__ actually set, not a hardcoded list. Simulate a future donfig + # that grows a field: the derived allowlist absorbs it, where a + # hardcoded list would start rejecting donfig's own writes. + import donfig + + real_init = donfig.Config.__init__ + + def init_with_extra_field(self, *args, **kwargs): + real_init(self, *args, **kwargs) + self.hypothetical_future_field = 1 + + monkeypatch.setattr(donfig.Config, "__init__", init_with_extra_field) + probe = type(gb.config)("test_derived_allowlist") + assert "hypothetical_future_field" in probe._donfig_attrs + probe.hypothetical_future_field = 2 # writable, not rejected + with pytest.raises(AttributeError, match="autocompute"): + probe.autocompute = False