Skip to content

Raise when thermocycler hold time is unavailable#1010

Merged
rickwierenga merged 3 commits intoPyLabRobot:mainfrom
Abdeltoto:fix-thermocycler-hold-time-not-running
Apr 26, 2026
Merged

Raise when thermocycler hold time is unavailable#1010
rickwierenga merged 3 commits intoPyLabRobot:mainfrom
Abdeltoto:fix-thermocycler-hold-time-not-running

Conversation

@Abdeltoto
Copy link
Copy Markdown
Contributor

@Abdeltoto Abdeltoto commented Apr 26, 2026

Summary

  • Raise RuntimeError in the Opentrons HTTP thermocycler backend when hold time is unavailable because no profile is running.
  • Let is_profile_running() propagate backend RuntimeErrors instead of converting them to False.
  • Add tests for generic profile-running error propagation and the Opentrons HTTP backend behavior.

Fixes #635

Test plan

  • python -m ruff check pylabrobot/thermocycling/thermocycler.py pylabrobot/thermocycling/opentrons_backend.py pylabrobot/thermocycling/thermocycler_tests.py pylabrobot/thermocycling/opentrons_backend_tests.py
  • python -m pytest pylabrobot/thermocycling/thermocycler_tests.py::ThermocyclerTests::test_is_profile_running_raises_if_no_profile_running
  • python -m pytest pylabrobot/thermocycling/thermocycler_tests.py::ThermocyclerTests::test_is_profile_running_preserves_not_implemented_error
  • python -m pytest pylabrobot/thermocycling/opentrons_backend_tests.py::TestOpentronsThermocyclerBackend::test_get_hold_time_raises_if_not_running skipped locally because ot_api is not installed.

Copy link
Copy Markdown
Member

@rickwierenga rickwierenga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR! one small comment/ suggestion

Comment on lines +263 to +267
total_steps = await self.get_total_step_count(**backend_kwargs)
except NotImplementedError:
raise
except RuntimeError:
return False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to raise on RuntimeError here as well? False seems incorrect

@Abdeltoto
Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I updated this so \is_profile_running()\ now propagates the \RuntimeError\ instead of returning \False\, and adjusted the test accordingly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's maybe keep the return values in the chatterbox until we have a proper simulator for it. This breaks CI and potentially existing protocols.

@Abdeltoto
Copy link
Copy Markdown
Contributor Author

Abdeltoto commented Apr 26, 2026

Good point. I reverted the Chatterbox behavior and kept its existing 0.0 return for now, so this PR doesn’t affect simulator-style usage or existing protocols. The runtime error behavior is now limited to the Opentrons backend, and I updated the tests/PR summary accordingly.

@rickwierenga rickwierenga merged commit a207f3c into PyLabRobot:main Apr 26, 2026
21 checks passed
@rickwierenga
Copy link
Copy Markdown
Member

thanks!

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.

Thermocycler.get_hold_time should raise an error when no protocol is running

2 participants