Skip to content

Annotate (and decide on renaming) the builtin-shadowing Element id param (#7) #3

Description

@jawjay

Problem

Every element's public constructor takes an id parameter that has two small but real issues:

  1. Shadows the builtin id(). id is a Python builtin; using it as a parameter name is a readability/lint smell on the public surface users actually call.
  2. Loses its type annotation in subclasses. The base is annotated, the subclasses are not:
    • src/qtviz/core/element.py:53def __init__(self, *, backend_hint: str | None = None, id: ElementId | None = None) ✅ annotated
    • all 9 public constructors degrade it to bare id=None: scatter.py:34, curve.py:32, bars.py:29, histogram.py:26, errorbars.py:28, heatmap.py:28, image.py:25, spread.py:28, raw_figure.py:33

For a contract-first library ("the types are the documentation"), the public-facing constructors are exactly where the annotation should be present.

Two parts, different risk

(a) Re-add the annotation — non-breaking, do anytime.
Change bare id=None to id: ElementId | None = None in the 9 constructors. Runtime-invisible; purely improves the documented contract. Each file needs ElementId imported from ..core.element.

(b) Rename idelement_id — breaking, decide before 1.0.
Removes the builtin shadow but renames a public kwarg. Under Hyrum's Law, anyone passing id= (e.g. to carry a stable identity across rebuilds, as the raster pipeline does internally via id=node.id) breaks. Weigh value vs. churn. If done, do it pre-1.0 with a CHANGELOG note.

Recommendation

Land (a) opportunistically (it's trivial and safe — could even ride along with another elements touch). Treat (b) as a genuine pre-1.0 decision: the shadow is cosmetic, so "leave id as-is, just annotate it" is a defensible close. Capture the decision here either way.

Acceptance criteria

  • All 9 public element constructors annotate the id param (id: ElementId | None = None).
  • Decision recorded on whether to rename idelement_id (and done + CHANGELOG'd if yes, or closed as "won't rename" if no).
  • If renamed: internal callers updated (id=node.id in data/pipeline.py rasterization, any tests/examples).

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions