850 fix for exceptions being ignored in beamline unit tests

[rtuck99]

Fixes #850

Instructions to reviewer on how to test:

1. Unit tests all pass

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}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.54%. Comparing base (a3a3cf6) to head (a912bfb).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #868   +/-   ##
=======================================
  Coverage   95.54%   95.54%           
=======================================
  Files         125      125           
  Lines        5343     5344    +1     
=======================================
+ Hits         5105     5106    +1     
  Misses        238      238           

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

[DiamondJoseph]

Just a nit

[DiamondJoseph]

Nit: I'd prefer that Undulator took as a default argument id_gap_lookup_table_path=os.devnull: on i22, p38, the undulator isn't used as a Movable, and i22.py is pointing to an empty file on the dls filesystem. This would give us an empty file that is available everywhere, and when a beamline wants to specify it they can.
(splitting the Undulator into a Movable impl and a non-movable impl again is another issue)

[DominicOram]

I think all this device instantiation stuff has taught us we need a general look at how we provide these lookup tables to devices anyway. The undulator is maybe an exceptional case to this where actually in some places it's not changed, maybe we should have a writable and a read-only undulator

[rtuck99]

I will change it to os.devnull for now - I was basically looking here for someone to tell me what the correct path was

[DiamondJoseph]

nit: I disagree with this- I assume it's done because device_instantiation wants a prefix arg: there's 2 prefixes here, motion gives context to what it does.

[rtuck99]

This is the downside of python having both named and positional parameters - you don't get to choose the parameter names of your functions if you want them to be called by a framework :)

[DominicOram]

I agree that we should allow not forcing prefix but whilst the old instantiation method insists on it I think we have to have it

[DominicOram]

I think we should just do #871. I'll do that separate to this PR though

[DominicOram]

This is good, thanks. I agree that the SPECIFY_ME isn't great but I will make a new issue to holistically look at undulators

[fajinyuan]

Sorry to ask a silly question here - does P38 has undulator or insertion device?

________________________________ From: Dominic Oram ***@***.***> Sent: Friday, October 25, 2024 16:27 To: DiamondLightSource/dodal ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [DiamondLightSource/dodal] 850 fix for exceptions being ignored in beamline unit tests (PR #868) @DominicOram commented on this pull request.

________________________________ In src/dodal/beamlines/p38.py<#868 (comment)>:

@@ -253,6 +252,7 @@ def undulator(

f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:", wait_for_connection, fake_with_ophyd_sim, + id_gap_lookup_table_path="SPECIFY_ME", I think all this device instantiation stuff has taught us we need a general look at how we provide these lookup tables to devices anyway. The undulator is maybe an exceptional case to this where actually in some places it's not changed, maybe we should have a writable and a read-only undulator — Reply to this email directly, view it on GitHub<#868 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAAQXXIOJBYA7XX7ICUJ2KDZ5JPORAVCNFSM6AAAAABQS4Y7PGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOJVGY2TMNBXHA>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***> This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail. Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd. Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message. Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom.