Skip to content

Update to ophyd-async v0.20.1 - #2122

Merged
oliwenmandiamond merged 21 commits into
mainfrom
update_to_ophyd_async_0_20
Jul 29, 2026
Merged

Update to ophyd-async v0.20.1#2122
oliwenmandiamond merged 21 commits into
mainfrom
update_to_ophyd_async_0_20

Conversation

@oliwenmandiamond

@oliwenmandiamond oliwenmandiamond commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Update to latest ophyd-async version v0.20.1.

Summary of changes

  • Replaces Bluesky protected attribute assignment in tests with set_mock_attr
  • Fixes tests that failed due to signals not being named or devices not connected.

Instructions to reviewer on how to test:

1.Check tests pass
2. Check AI conversions of setting mock bluesky verbs done correctly

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

oliwenmandiamond and others added 5 commits July 22, 2026 13:24
ophyd-async 0.20.1 adds set_mock_attr (in ophyd_async.core) and enforces
that test code can no longer assign directly to attribute names that
collide with bluesky protocol verbs (set, read, trigger, kickoff,
complete, prepare, describe, check_value). Replace direct assignments
like `signal.set = AsyncMock(...)` with `set_mock_attr(signal, "set",
AsyncMock(...))` across the affected test files and fixtures.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQPRwf3pUved4ihrBLPi1g
The set_mock_attr fixes in the previous commit cover every reserved-attr
NameError that this autouse fixture was blanket-suppressing, so the
mask is no longer needed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQPRwf3pUved4ihrBLPi1g

coretl commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

ophyd-async 0.20.1 set_mock_attr fixes

Bumped the ophyd-async[ca,pva] pin to >=0.20.1 (now resolved in uv.lock) and fixed the remaining reserved-attr test failures using the new set_mock_attr helper (imported from ophyd_async.core — it isn't re-exported from ophyd_async.testing in this release).

Tests fixed with set_mock_attr (direct signal.set = mock/.read =/.trigger =/etc. assignments replaced with set_mock_attr(signal, "attr", mock)):

  • tests/devices/beamlines/i03/test_beamsize.py, i03/test_undulator_dcm.py, i04/test_beamsize.py
  • tests/devices/beamlines/i06_shared/test_i06_apple2controller.py, i09_2/test_i09_2_motors.py, i10/test_i10_apple2.py, i15_1/test_hexapod.py, i17/test_i17_apple2.py, i21/test_toolpoint_motion.py
  • tests/devices/electron_analyser/base/test_detector_logic.py
  • tests/devices/insertion_device/test_apple2_controller.py, test_apple2_undulator.py, test_energy.py, test_polarisation.py
  • tests/devices/oav/conftest.py, tests/devices/oav/test_oav.py
  • tests/devices/test_attenuator.py, test_bart_robot.py, test_common_mirrors.py, test_focusing_mirror.py, test_gridscan.py, test_smargon.py, test_zebra.py, test_zocalo_results.py

Also removed the temporary OPHYD_ASYNC_ALLOW_RESERVED_ATTRS=YES autouse fixture from tests/conftest.py added in a previous commit — the set_mock_attr fixes above cover every case it was masking, confirmed by re-running the full suite with the mask removed (identical failure set before/after).

Remaining failures left for a human (all pre-existing, none addressable by set_mock_attr):

  • 56 tests/devices/beamlines/{i09_2_shared,i10}/test_i09_apple2.py / test_i10_apple2.py cases — TypeError: mock_config_client.<locals>.my_side_effect() missing 1 required positional argument: 'desired_return_type'. This is a second, still-broken duplicate mock_config_client fixture in src/dodal/testing/fixtures/devices/hard_undulator.py (positional-only my_side_effect(file_path, desired_return_type, reset_cached_result)) — the "Fix ConfigClient issue" commit only fixed the copy in src/dodal/testing/fixtures/devices/apple2.py. Likely just needs the same default-args treatment applied there.
  • 1 tests/devices/beamlines/i15_1/test_robot.py::test_puck_program_loaded_before_position_selected — EPICS/IOC timeout, infra-related.

Full suite: 3268 passed, 58 failed (all the mock_config_client cases above), 1 skipped.


Generated by Claude Code

set_mock_attr(device, "attr", mock) doesn't let pyright narrow
device.attr to the mock's type, so later device.attr.assert_*() calls
failed with reportAttributeAccessIssue. Capture set_mock_attr's return
value (it returns the mock unchanged, per its own docstring) and assert
on that instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQPRwf3pUved4ihrBLPi1g

coretl commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Fixed a pyright regression from the set_mock_attr change: set_mock_attr(device, "attr", mock) doesn't let pyright narrow device.attr to the mock's type, so later device.attr.assert_*(...) calls were failing type-checking with reportAttributeAccessIssue. Fixed by capturing set_mock_attr's return value (it returns the mock unchanged, per its own docstring) and asserting on that instead, across the 14 affected test files. pyright and the full test suite are clean again (same 58 pre-existing mock_config_client failures as before, nothing new).


Generated by Claude Code

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.16%. Comparing base (50b6b22) to head (e5bc9a5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2122      +/-   ##
==========================================
- Coverage   99.16%   99.16%   -0.01%     
==========================================
  Files         354      354              
  Lines       13849    13848       -1     
==========================================
- Hits        13733    13732       -1     
  Misses        116      116              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oliwenmandiamond
oliwenmandiamond marked this pull request as ready for review July 29, 2026 09:30
@oliwenmandiamond
oliwenmandiamond requested a review from a team as a code owner July 29, 2026 09:30
@oliwenmandiamond oliwenmandiamond changed the title Update to ophyd-async v0.20 Update to ophyd-async v0.20.1 Jul 29, 2026

coretl commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

CI is fully green now — the "Standardise config client" merge (#2099) replaced the duplicate mock_config_client fixture mechanism and fixed the remaining 56/58 failures I'd flagged as out-of-scope. Confirmed locally on 6b424b6: 3375 passed, 1 skipped, 0 failed; pre-commit, ruff, and pyright all clean. All GitHub Actions checks (lint, tests × 4 Python versions, docs, dist, codecov) are passing.


Generated by Claude Code

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, generally looks good. Just the one query

Comment thread src/dodal/devices/insertion_device/apple2_controller.py

@Relm-Arrowny Relm-Arrowny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good overall, just one small thing to address before merging!

Could you also add a note to the PR description outlining the changes?
Something like:
"Replaces Bluesky protected attribute assignment in tests with set_mock_attr"

Comment thread tests/devices/beamlines/i10_1/high_field_magnet/test_high_field_magnet.py Outdated
user_readback defaulted to 0 and was never set to a distinct value, so
the assertion trivially passed regardless of what stop() did. Set
setpoint and readback to different values so the test actually checks
that stop() resets the setpoint to the current readback.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQPRwf3pUved4ihrBLPi1g

coretl commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Addressed both review notes:

  • Added a "Summary of changes" section to the PR description per @Relm-Arrowny's request.
  • Fixed test_stop_success in tests/devices/beamlines/i10_1/high_field_magnet/test_high_field_magnet.py per @Relm-Arrowny's suggestion — user_readback was never set away from its default 0, so the test passed trivially regardless of stop()'s behaviour. Now sets user_setpoint/user_readback to distinct values so the assertion actually exercises the reset logic.

Generated by Claude Code

coretl commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

FYI: test (ubuntu-latest, 3.11) failed once on 93b717b with test_given_program_not_running_and_pin_unmounting_but_new_pin_not_mounting_when_load_pin_then_timeout (AssertionError: Expected 'mock' to have been called once. Called 0 times.), while 3.12/3.13/3.14 passed on the same commit and it passes consistently locally (5 isolated runs + full suite). Looks like pre-existing timing flakiness from this test's tight timeouts (_set_fast_robot_timeouts) rather than a real regression — not something to chase from this PR.


Generated by Claude Code

Comment thread src/dodal/devices/insertion_device/apple2_controller.py
@oliwenmandiamond
oliwenmandiamond enabled auto-merge (squash) July 29, 2026 13:15
@oliwenmandiamond
oliwenmandiamond merged commit c5a4fc3 into main Jul 29, 2026
10 checks passed
@oliwenmandiamond
oliwenmandiamond deleted the update_to_ophyd_async_0_20 branch July 29, 2026 13:22
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.

5 participants