Skip to content

Commit c14c5e2

Browse files
author
Matt Davis
committed
[IRON] Document parent-constructor bypass in AccumFifoHandle / PacketFifoHandle
Address review issue 7: AccumFifoHandle and PacketFifoHandle deliberately do not call super().__init__() because ObjectFifoHandle's constructor requires an ObjectFifo with semantics (depth, dims_from_stream_per_cons, _get_endpoint) that AccumFifo and PacketFifo do not have. They instead stub the attributes ObjectFifoHandle exposes as properties directly. Add an explicit class-docstring '.. note::' section in each documenting: - which attributes are stubbed - why super() is bypassed - that all_of_endpoints (which traverses _object_fifo) is overridden - that the proper long-term fix is a shared narrower base class This is a documentation fix, not a behavioural change. The shared base class refactor is intentionally out of scope for the initial primitive landing; the bypass pattern is rare in upstream IRON code, and explicit documentation keeps the next maintainer from being surprised. (SparseFifoHandle and VariableRateFifoHandle inherit normally from ObjectFifoHandle and do call super().__init__; only AccumFifoHandle and PacketFifoHandle exhibit the bypass pattern.)
1 parent 2594626 commit c14c5e2

2 files changed

Lines changed: 30 additions & 9 deletions

File tree

python/iron/accum.py

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,14 +478,30 @@ class AccumFifoHandle(ObjectFifoHandle):
478478
:class:`AccumFifo`'s point-to-point invariant in the cascade case
479479
(a cascade transfer takes exactly one accumulator per cycle; there
480480
is no notion of a "circular buffer with depth N" here).
481+
482+
.. note::
483+
:meth:`__init__` deliberately does **not** call
484+
``super().__init__()`` -- :class:`ObjectFifoHandle`'s constructor
485+
requires an ``of`` argument with ObjectFifo semantics (depth,
486+
``dims_from_stream_per_cons``, ``_get_endpoint`` etc.) that
487+
:class:`AccumFifo` does not have. We instead stub the attributes
488+
:class:`ObjectFifoHandle` exposes as properties (``_port``,
489+
``_is_prod``, ``_depth``, ``_endpoint``, ``_dims_from_stream``,
490+
``_tile``) directly so isinstance-based downstream code keeps
491+
working. This pattern is repeated in :class:`PacketFifoHandle`,
492+
:class:`SparseFifoHandle`, and :class:`VariableRateFifoHandle`;
493+
a shared narrower base class is the intended long-term fix
494+
(see the "Future work" note in module docstrings) but is out of
495+
scope for the initial primitive landing.
496+
497+
Methods inherited from :class:`ObjectFifoHandle` that traverse
498+
``self._object_fifo`` (notably :meth:`all_of_endpoints`) are
499+
overridden below so the bypass does not produce
500+
``AttributeError`` at lowering time.
481501
"""
482502

483503
def __init__(self, accum_fifo: AccumFifo, is_prod: bool):
484-
# Bypass ObjectFifoHandle.__init__ -- it requires an `of` with
485-
# ObjectFifo semantics (depth, dims_from_stream_per_cons, etc.)
486-
# that AccumFifo does not have. Set the fields ObjectFifoHandle
487-
# exposes as properties directly so isinstance-based downstream
488-
# code keeps working.
504+
# See class docstring "note" for why super().__init__() is bypassed.
489505
self._port = (
490506
ObjectFifoPort.Produce if is_prod else ObjectFifoPort.Consume
491507
)

python/iron/packet.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,14 @@ class PacketFifoHandle(ObjectFifoHandle):
609609
:meth:`send_with_header` and :meth:`recv_header` are added on top
610610
for callers that need explicit per-packet header control (the
611611
filter-early use case).
612+
613+
.. note::
614+
:meth:`__init__` bypasses ``super().__init__()`` for the same
615+
reason :class:`AccumFifoHandle` does -- :class:`ObjectFifoHandle`
616+
requires an ``of`` with ObjectFifo semantics that
617+
:class:`PacketFifo` does not have. See the corresponding note in
618+
:class:`AccumFifoHandle`. A shared narrower base class is the
619+
intended long-term fix.
612620
"""
613621

614622
def __init__(
@@ -618,10 +626,7 @@ def __init__(
618626
index: int,
619627
depth: int,
620628
):
621-
# Bypass ObjectFifoHandle.__init__ (it requires an `of` with
622-
# ObjectFifo semantics: depth, dims_from_stream_per_cons, etc.)
623-
# and set the fields ObjectFifoHandle exposes as properties
624-
# directly so isinstance-based downstream code keeps working.
629+
# See class docstring "note" for why super().__init__() is bypassed.
625630
self._port = (
626631
ObjectFifoPort.Produce if is_prod else ObjectFifoPort.Consume
627632
)

0 commit comments

Comments
 (0)