Skip to content

refactor(controllers): constructor-pass path, drop set_path() #373

@gilesknap

Description

@gilesknap

Proposal (2d)

Make path a parameter of Controller.__init__ and thread it through user code. Remove set_path() entirely. The launcher constructs the root with cls(path=[entry.id], **options); a parent constructs its subs with the full path baked in:

class Temp(Controller):
    def __init__(self, path: list[str], settings: TempSettings):
        super().__init__(path=path)
        self.add_sub_controller("ramp_0", Ramp(path=path + ["ramp_0"], ...))

add_sub_controller(name, sub) no longer mutates sub._path — it sanity-asserts sub.path[-1] == name (or just trusts the parent).

Result: one concept (path), no post-construction setters, no rootless-sub state, no two-phase init, no recursive re-prefix.

Cost

Any custom Controller.__init__ (root and sub-controllers) must accept path and forward to super:

def __init__(self, path: list[str], my_settings: MySettings):
    super().__init__(path=path)

This is the same ergonomic shape as the rejected branch B (yaml-list-id-positional), applied to every customised Controller, not just roots. We're accepting it here because path is a real Controller concept, not a YAML-leaked one — user code already touches self.path + [name] when adding subs, so threading path through __init__ is paying the cost where the concept naturally lives. That distinguishes it from id, which was a YAML label that shouldn't have leaked into user signatures (which is what motivated branch C).

The contrarian read: "2d is just branch B with path instead of id" — mechanically true. Be honest about that.

Open sub-decisions

  • path positional vs keyword-only on Controller.__init__. Lean kwonly (matches branch A) to avoid the sub-controller call-site fallout that branch B hit (description= had to become a kwarg to dodge the new positional slot).
  • Does path default to [] for direct/standalone construction (tests, embedded use)? Probably yes.
  • add_sub_controller's residual role: pure registration + sanity-assert, or drop it and have parents just write self.sub_controllers[name] = ...?

Context: how we got here

Branch C (landed in PR #360 as c8adfee7) drops _id / set_id / Controller.id. The launcher does c = cls(options); c.set_path([entry.id]). The single production reader of .id (control_system.py:26, IPython context-key + dup-key validation) reads path[0] instead. YAML id: becomes purely a label for the root path segment.

Branch C deliberately leaves one wrinkle alone: when a root Controller adds subs in its __init__, those subs' _path is set to [name] (rootless), because the launcher hasn't seeded the root's _path yet. Today's _build_api papers over this by recursing with path + [name] from the root's seeded path, so emitted ControllerAPI.paths come out correct — but sub_controller._path itself remains rootless. That latent oddity exists on multiple-controllers today and has never bitten anyone; bundling its fix into branch C would obscure whether the id-removal itself was the right call. Hence this issue.

Alternatives considered for fixing the rootless-sub state

  • 2a — Two-phase Controller lifecycle. Split sub-registration out of __init__ into a framework-called register_subs() hook that runs after set_path. Forces every existing controller to migrate add_sub_controller calls out of __init__. Rejected: high churn for a problem users don't currently feel.
  • 2b — Declarative sub-controllers. Class-level descriptors / spec returned from __init__, framework instantiates with full path baked in. Rejected: framework machinery, awkward for runtime-determined subs (e.g. count from settings).
  • 2c — Recursive re-prefix on set_path. Make set_path walk descendants and re-prefix their _paths. Local change, invisible to users, preserves current authoring style. Plausible but keeps a post-construction setter that 2d eliminates entirely.
  • 2d (this proposal). Constructor-pass path. Drops the setter; subs constructed under a populated self.path are correct first time; no recursion needed.

2d wins because it's the only option that removes the post-construction setter and the rootless-sub state simultaneously. The cost (user __init__ boilerplate for path) is the price.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions