Skip to content

Commit dda3378

Browse files
authored
Experimental API for port and param docs (#343)
Adds an optional `doc='...'` argument to `self.Port(...)`, `self.Parameter(...)` and `self.ArgParameter(...)`. Docs are stored in the block, similar to how optional and port tags are tracked. The idea is this keeps documentation close to the port definition and (in the future) allows tools that are port-/parameter-aware to generate documentation specific to the data model here. Python doesn't have a accepted convention for class attribute docs, so we're going to make our own. [PEP 224](https://peps.python.org/pep-0224/) was a proposal for attribute docstrings, but was rejected. Quick informal conventions: - Type information should not be repeated in the docstring. - Other arguments to Port should not be repeated in the docstring (e.g., optional, port tags). Tools should automatically add these when generating visualizations / documentation. - Documentation should not simply repeat the port name, but rather add more information / context - `pwr`, `gnd` would not need docstrings unless the docstrings add information, e.g. disambiguate between multiple power rails or `pwr_in` vs `pwr_out` or highlight odd behavior.
1 parent da7a2ea commit dda3378

File tree

2 files changed

+32
-18
lines changed

2 files changed

+32
-18
lines changed

edg_core/Blocks.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,12 @@ def __init__(self) -> None:
244244

245245
# TODO delete type ignore after https://github.com/python/mypy/issues/5374
246246
self._parameters: SubElementDict[ConstraintExpr] = self.manager.new_dict(ConstraintExpr) # type: ignore
247+
self._param_docs = IdentityDict[ConstraintExpr, str]()
248+
247249
self._ports: SubElementDict[BasePort] = self.manager.new_dict(BasePort) # type: ignore
248250
self._required_ports = IdentitySet[BasePort]()
251+
self._port_docs = IdentityDict[BasePort, str]()
252+
249253
self._connects = self.manager.new_dict(Connection, anon_prefix='anon_link')
250254
self._connects_by_port = IdentityDict[BasePort, Connection]() # port -> connection
251255
self._connect_delegateds = IdentityDict[Connection, List[Connection]]() # for net joins, joined connect -> prior connects
@@ -474,7 +478,7 @@ def assign(self, target: ConstraintExpr[ConstrType, Any],
474478
return constraint
475479

476480
T = TypeVar('T', bound=BasePort)
477-
def Port(self, tpe: T, *, optional: bool = False) -> T:
481+
def Port(self, tpe: T, *, optional: bool = False, doc: Optional[str] = None) -> T:
478482
"""Registers a port for this Block"""
479483
if self._elaboration_state != BlockElaborationState.init:
480484
raise BlockDefinitionError(self, "can't call Port(...) outside __init__",
@@ -488,10 +492,13 @@ def Port(self, tpe: T, *, optional: bool = False) -> T:
488492
if not optional:
489493
self._required_ports.add(elt)
490494

495+
if doc is not None:
496+
self._port_docs[elt] = doc
497+
491498
return elt
492499

493500
ConstraintType = TypeVar('ConstraintType', bound=ConstraintExpr)
494-
def Parameter(self, tpe: ConstraintType) -> ConstraintType:
501+
def Parameter(self, tpe: ConstraintType, *, doc: Optional[str] = None) -> ConstraintType:
495502
"""Registers a parameter for this Block"""
496503
if self._elaboration_state != BlockElaborationState.init:
497504
raise BlockDefinitionError(self, "can't call Parameter(...) outside __init__",
@@ -504,6 +511,9 @@ def Parameter(self, tpe: ConstraintType) -> ConstraintType:
504511
if elt.initializer is not None:
505512
self._check_constraint(elt.initializer)
506513

514+
if doc is not None:
515+
self._param_docs[elt] = doc
516+
507517
return elt
508518

509519
def connect(self, *connects: Union[BasePort, Connection], flatten=False) -> Connection:

edg_core/HierarchyBlock.py

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -455,27 +455,27 @@ def connect(self, *connects: Union[BasePort, Connection], flatten=False) -> Conn
455455
from .ConstraintExpr import BoolLike, BoolExpr, FloatLike, FloatExpr, RangeLike, RangeExpr
456456
# type ignore is needed because IntLike overlaps BoolLike
457457
@overload
458-
def ArgParameter(self, param: BoolLike) -> BoolExpr: ... # type: ignore
458+
def ArgParameter(self, param: BoolLike, *, doc: Optional[str] = None) -> BoolExpr: ... # type: ignore
459459
@overload
460-
def ArgParameter(self, param: IntLike) -> IntExpr: ... # type: ignore
460+
def ArgParameter(self, param: IntLike, *, doc: Optional[str] = None) -> IntExpr: ... # type: ignore
461461
@overload
462-
def ArgParameter(self, param: FloatLike) -> FloatExpr: ... # type: ignore
462+
def ArgParameter(self, param: FloatLike, *, doc: Optional[str] = None) -> FloatExpr: ... # type: ignore
463463
@overload
464-
def ArgParameter(self, param: RangeLike) -> RangeExpr: ... # type: ignore
464+
def ArgParameter(self, param: RangeLike, *, doc: Optional[str] = None) -> RangeExpr: ... # type: ignore
465465
@overload
466-
def ArgParameter(self, param: StringLike) -> StringExpr: ... # type: ignore
466+
def ArgParameter(self, param: StringLike, *, doc: Optional[str] = None) -> StringExpr: ... # type: ignore
467467
@overload
468-
def ArgParameter(self, param: ArrayBoolLike) -> ArrayBoolExpr: ... # type: ignore
468+
def ArgParameter(self, param: ArrayBoolLike, *, doc: Optional[str] = None) -> ArrayBoolExpr: ... # type: ignore
469469
@overload
470-
def ArgParameter(self, param: ArrayIntLike) -> ArrayIntExpr: ... # type: ignore
470+
def ArgParameter(self, param: ArrayIntLike, *, doc: Optional[str] = None) -> ArrayIntExpr: ... # type: ignore
471471
@overload
472-
def ArgParameter(self, param: ArrayFloatLike) -> ArrayFloatExpr: ... # type: ignore
472+
def ArgParameter(self, param: ArrayFloatLike, *, doc: Optional[str] = None) -> ArrayFloatExpr: ... # type: ignore
473473
@overload
474-
def ArgParameter(self, param: ArrayRangeLike) -> ArrayRangeExpr: ... # type: ignore
474+
def ArgParameter(self, param: ArrayRangeLike, *, doc: Optional[str] = None) -> ArrayRangeExpr: ... # type: ignore
475475
@overload
476-
def ArgParameter(self, param: ArrayStringLike) -> ArrayStringExpr: ... # type: ignore
476+
def ArgParameter(self, param: ArrayStringLike, *, doc: Optional[str] = None) -> ArrayStringExpr: ... # type: ignore
477477

478-
def ArgParameter(self, param: CastableType) -> ConstraintExpr[Any, CastableType]:
478+
def ArgParameter(self, param: CastableType, *, doc: Optional[str] = None) -> ConstraintExpr[Any, CastableType]:
479479
"""Registers a constructor argument parameter for this Block.
480480
This doesn't actually do anything, but is needed to help the type system converter the *Like to a *Expr."""
481481
if not isinstance(param, ConstraintExpr):
@@ -484,23 +484,27 @@ def ArgParameter(self, param: CastableType) -> ConstraintExpr[Any, CastableType]
484484
raise TypeError(f"param to ArgParameter(...) must have binding")
485485
if not isinstance(param.binding, InitParamBinding):
486486
raise TypeError(f"param to ArgParameter(...) must be __init__ argument with @init_in_parent")
487+
488+
if doc is not None:
489+
self._param_docs[param] = doc
490+
487491
return param
488492

489493
T = TypeVar('T', bound=BasePort)
490-
def Port(self, tpe: T, tags: Iterable[PortTag]=[], *, optional: bool = False) -> T:
494+
def Port(self, tpe: T, tags: Iterable[PortTag]=[], *, optional: bool = False, doc: Optional[str] = None) -> T:
491495
"""Registers a port for this Block"""
492496
if not isinstance(tpe, (Port, Vector)):
493497
raise NotImplementedError("Non-Port (eg, Vector) ports not (yet?) supported")
494498
for tag in tags:
495499
assert_cast(tag, PortTag, "tag for Port(...)")
496500

497-
port = super().Port(tpe, optional=optional)
501+
port = super().Port(tpe, optional=optional, doc=doc)
498502

499503
self._port_tags[port] = set(tags)
500504
return port # type: ignore
501505

502506
ExportType = TypeVar('ExportType', bound=BasePort)
503-
def Export(self, port: ExportType, tags: Iterable[PortTag]=[], *, optional: bool = False) -> ExportType:
507+
def Export(self, port: ExportType, tags: Iterable[PortTag]=[], *, optional: bool = False, doc: Optional[str] = None) -> ExportType:
504508
"""Exports a port of a child block, but does not propagate tags or optional."""
505509
assert port._is_bound(), "can only export bound type"
506510
port_parent = port._block_parent()
@@ -510,10 +514,10 @@ def Export(self, port: ExportType, tags: Iterable[PortTag]=[], *, optional: bool
510514
if isinstance(port, BaseVector): # TODO can the vector and non-vector paths be unified?
511515
assert isinstance(port, Vector)
512516
new_port: BasePort = self.Port(Vector(port._tpe.empty()),
513-
tags, optional=optional)
517+
tags, optional=optional, doc=doc)
514518
elif isinstance(port, Port):
515519
new_port = self.Port(type(port).empty(), # TODO is dropping args safe in all cases?
516-
tags, optional=optional)
520+
tags, optional=optional, doc=doc)
517521
else:
518522
raise NotImplementedError(f"unknown exported port type {port}")
519523

0 commit comments

Comments
 (0)