Skip to content

Use ActionInfo in Invocations - #414

Merged
bprobert97 merged 3 commits into
mainfrom
dont-pass-around-actiondescriptor
Nov 3, 2026
Merged

bprobert97 merged 3 commits into
mainfrom
dont-pass-around-actiondescriptor

Conversation

@rwb27

@rwb27 rwb27 commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Passing around descriptor instances is hard to type properly and gets really confusing.

I've changed Invocation so that action is an ActionInfo object rather than an ActionDescriptor, which should fix that problem.

I've also changed list_invocations to use thing and action names, instead of objects.

Closes #403

This MR contains the following

  • Store an ActionInfo object in Invocations, rather than weak references to a Thing and an ActionDescriptor.
  • Change argument types of invoke_action and list_invocations to match.

Merge checklist:

  • All new/changed functions have up to date typehints and docstrings.
  • Any changes to the public API have been updated in docs/src/public_api.rst.
  • New or changed features have been added to (or updated in) the conceptual documentation.
  • Any new or updated dependencies have been added to the project configuration (e.g., pyproject.toml).
  • New functionality is fully tested.
  • Any decrease in test coverage has been justified.
  • Either the test-against-ofm-v3 job passes, or test-against-ofm-feature-branch passes.
  • New features have been used in a branch of the OpenFlexure Microscope, which is tested in test-against-ofm-feature-branch.
  • This code has been tested manually against simulated hardware (detail tests in "Before merge").
  • This code has been tested against real hardware (detail tests in "Before merge").

Passing around descriptor instances is hard to type properly and gets really confusing.

I've changed `Invocation` so that `action` is an `ActionInfo` object rather than an `ActionDescriptor`, which should fix that problem.

I've also changed `list_invocations` to use thing and action names, instead of objects.
@barecheck

barecheck Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Barecheck - Code coverage report

Total: 97.42%

Your code coverage diff: 0.00% ▴

Uncovered files and lines
FileLines
src/labthings_fastapi/actions.py585-586, 599, 635-636, 837, 977-978, 981, 984, 1035

Added a test for the new property, which also checks a few other features of Invocation.
@rwb27
rwb27 requested a review from bprobert97 November 2, 2026 09:23
Comment thread src/labthings_fastapi/actions.py Resolved
Comment thread tests/test_actions.py Resolved
Comment thread tests/test_actions.py Resolved
This removes a left-over print statement and swaps `is` for `==` in a string comparison.
It also adds a timeout to a test that had the potential to hang indefinitely.
@rwb27
rwb27 force-pushed the dont-pass-around-actiondescriptor branch from bdf8d9f to dae42db Compare November 2, 2026 21:12
@rwb27
rwb27 requested a review from bprobert97 November 2, 2026 21:55
@bprobert97
bprobert97 merged commit 6c415be into main Nov 3, 2026
14 checks passed
@bprobert97
bprobert97 deleted the dont-pass-around-actiondescriptor branch November 3, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect types in for Invocation.action

2 participants