Will
August 10, 2026, 10:29pm
1
Hi All!
I wanted to drop a quick hello as I just submitted my first PR.
main ← WillVT84:fix/star-deck-trash96-pointer
opened 09:58PM - 10 Aug 26 UTC
## Problem
`HamiltonSTARDeck._trash96` is assigned only in `__init__`, and `get… _trash_area96()` returns it directly. That cached reference does not survive the deck being rebuilt or the trash being unassigned, which produces two distinct failures.
**1. Deserialized decks raise even though the trash is present.** `serialize()` encodes the 96 trash as a child resource and emits `with_trash96=False`. A deck restored through `Deck.deserialize()` therefore runs `__init__` with the flag off and leaves `_trash96` as `None`, while `trash_core96` is re-attached from the serialized children. The resource is on the deck and correctly positioned, but the getter raises:
```python
deck2 = Deck.deserialize(STARLetDeck().serialize())
"trash_core96" in [c.name for c in deck2.children] # True
deck2.get_trash_area96()
# RuntimeError: Trash area for 96-well plates was not created. Initialize with `with_trash96=True`.
```
Anyone loading a saved layout has to re-establish the pointer by hand.
**2. `clear(include_trash=True)` leaves a stale pointer.** `clear()` unassigns the trash but never clears the attribute, so the getter hands back an orphaned `Trash` with `parent = None`. That fails later and further from the cause than an outright error would.
## Fix
Resolve the resource by name against the child tree, mirroring how the base class's `Deck.get_trash_area()` already works, and only trust the cached reference while it is still assigned to this deck. The `parent is self` guard is what fixes the second case: an unassigned trash falls through to the lookup and then raises honestly.
Behaviour when the trash genuinely was not created (`with_trash96=False`) is unchanged.
## Tests
Two regression tests in `hamilton_deck_tests.py`, one per failure mode. Both were confirmed to fail against the unfixed source — the round-trip test errors with the original `RuntimeError`, and the `include_trash` test fails with "RuntimeError not raised" — and to pass with the fix.
## Verification
- `pytest pylabrobot/resources` — 225 passed. The 5 failures are pre-existing Opentrons deck tests that fetch labware over the network and hit `SSL: CERTIFICATE_VERIFY_FAILED` locally; identical on baseline.
- `ruff check` / `ruff format --check` (0.15.4, per the pin) — clean on both changed files.
- `mypy` (1.18.2, per the pin) — `Success: no issues found in 2 source files`.
## Note
Not addressed here, but the underlying design smell is that `serialize()` writes `with_trash`/`with_trash96`/`core_grippers` as `False`/`None` because the data lives in the children — the "not very pretty to have this key though" comments. That constructor-flags-vs-children mismatch is what produced this bug, and may affect other cached references the same way. Happy to follow up separately if that's of interest.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
We’re fully ported over to PLR with our ML STAR and am quickly building up our system architecture. I’ve been coding around encountered issues so far, but figured I’d start pushing out small bug fixes and resource additions as they come up so I can clear them out of our implementation wrappers. We will probably have something far larger down the line, but for now it’ll just be small stuff. Let me know if I should be re-directing these to another branch or something. I’m new to open source projects!
2 Likes
thanks for the PR, responded on GitHub
love it, thank you! I’ll be faster to respond in the future…