Skip to content

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

Open
eriknw wants to merge 1 commit into
34-mask-guard-dup-updatefrom
35-ssconfig-attr-guard
Open

Make ss config attribute assignment raise instead of silently no-op#615
eriknw wants to merge 1 commit into
34-mask-guard-dup-updatefrom
35-ssconfig-attr-guard

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

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.


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 35-ssconfig-attr-guard branch from 3cf03a5 to dc8a036 Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch 3 times, most recently from ee88830 to 47d8569 Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch from 47d8569 to b47efad Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch 2 times, most recently from 417c942 to 9aa5c77 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch from 9aa5c77 to e31ffce Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch from e31ffce to a7aa796 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch from a7aa796 to 5842261 Compare August 6, 2026 20:36
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch 2 times, most recently from 3bfd685 to ee9c663 Compare August 7, 2026 02:49
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.
@eriknw
eriknw force-pushed the 35-ssconfig-attr-guard branch from ee9c663 to 4a7395f 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