DeviceManager adds device doc string to factory method #1829

oliwenmandiamond · 2026-01-20T13:49:49Z

Fixes #780

Examples

Device without documentation

>>> help(beamsize)
Help on DeviceFactory in module dodal.beamlines.i03:

beamsize = <beamsize: DeviceFactory(aperture_scatterguard: ...terguard) -> dodal.devices.i03.beamsize.Beamsize>
    Beamsize:
    
    No documentation available.

>>>

Device with documentation

>>> help(robot)
Help on DeviceFactory in module dodal.beamlines.i03:

robot = <robot: DeviceFactory() -> dodal.devices.robot.BartRobot>
    BartRobot:
    
    The sample changing robot.

>>>

Device with factory documentation and device documentation

>>> help(panda_fast_grid_scan)
Help on DeviceFactory in module dodal.beamlines.i03:

panda_fast_grid_scan = <panda_fast_grid_scan: DeviceFactory() -> dodal.devices.fast_grid_scan.PandAFastGridScan>
    This is used instead of the zebra_fast_grid_scan device when using the PandA.
    
    PandAFastGridScan:
    
    Device for panda constant-motion scan.
    
        In this scan, the goniometer's velocity
        is constant through each row. It doesn't slow down when going through trigger points.

>>>

Instructions to reviewer on how to test:

Check new tests make sense
Check tests 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}

DominicOram · 2026-01-20T13:51:48Z

I was literally just thinking of doing this

oliwenmandiamond · 2026-01-20T13:59:41Z

I was literally just thinking of doing this

Saved you the work 😄

dan-fernandes

Looks good once the failing tests are sorted

src/dodal/device_manager.py

codecov · 2026-01-20T15:05:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.05%. Comparing base (d8e8aab) to head (6ada650).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1829      +/-   ##
==========================================
- Coverage   99.06%   99.05%   -0.01%     
==========================================
  Files         290      290              
  Lines       11092    11107      +15     
==========================================
+ Hits        10988    11002      +14     
- Misses        104      105       +1

☔ View full report in Codecov by Sentry.
📢 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.

src/dodal/device_manager.py

…com/DiamondLightSource/dodal into device_manager_adds_device_doc_string

tpoliaw · 2026-01-21T13:16:23Z

tests/test_device_manager.py

+
+    assert OphydV2Device.__doc__ is not None
+    assert (
+        foo.__doc__ == f"{OphydV2Device.__name__}:\n\n{cleandoc(OphydV2Device.__doc__)}"


(here and below)
Replicating the implementation using f-strings here doesn't really test anything. It would be better if the expected values were normal literals.

To avoid depending on the docs of Device, it might be useful to have a DocsDevice to go with the NoDocsDevice with docs that won't change.

I've reused the _type_docs function in the tests which I think makes it much cleaner now

It still defeats the point of the test though. They're now effectively assert make_docs(func) == make_docs(func)

I disagree, it still checks to make sure the formatting of the docs is still called through this function.

Would me adding another test specifically for _type_docs works as expected be suitable enough?

It would help, but raises the question of whether private functions should be tested directly or only via the things that call them. I still think it would be useful for the tests to include an example of what the full docs should look like but I'll leave it up to you.

I've added the additional tests. I personally think it is sufficient but let me know what you think

src/dodal/device_manager.py

tests/test_device_manager.py

Co-authored-by: Peter Holloway <peter.holloway@diamond.ac.uk>

tpoliaw · 2026-01-21T14:33:28Z

src/dodal/device_manager.py

+    return _type_docs(return_type)
+
+
+def _type_docs(target: type[V1 | V2], extra_docs: str = "") -> str:


By making this take extra_docs the type_docs name is no longer correct and it re-introduces another string comparison. Is there a benefit to moving the concatenation out of the format method?

Yes, it makes the function much more reusable with the tests

See the other comment about testing private functions.

…com/DiamondLightSource/dodal into device_manager_adds_device_doc_string

oliwenmandiamond added 2 commits January 20, 2026 12:17

DeviceManager adds device doc string

01d1231

Improve DeviceManager to include device docs for factory methods

8474540

oliwenmandiamond requested a review from a team as a code owner January 20, 2026 13:49

Merge branch 'main' into device_manager_adds_device_doc_string

f9578dc

oliwenmandiamond changed the title ~~Device manager adds device doc string to factory method~~ DeviceManager adds device doc string to factory method Jan 20, 2026

oliwenmandiamond requested review from DominicOram, dan-fernandes and tpoliaw January 20, 2026 13:50

Remove Device from import

39ee8ab

dan-fernandes approved these changes Jan 20, 2026

View reviewed changes

dan-fernandes reviewed Jan 20, 2026

View reviewed changes

Fixed tests

e698564

tpoliaw requested changes Jan 20, 2026

View reviewed changes

src/dodal/device_manager.py Outdated Show resolved Hide resolved

src/dodal/device_manager.py Outdated Show resolved Hide resolved

src/dodal/device_manager.py Outdated Show resolved Hide resolved

src/dodal/device_manager.py Outdated Show resolved Hide resolved

DominicOram mentioned this pull request Jan 20, 2026

Add a dodal describe ixx to get information about what is on the beamline #1831

Open

tpoliaw reviewed Jan 20, 2026

View reviewed changes

src/dodal/device_manager.py Outdated Show resolved Hide resolved

DominicOram mentioned this pull request Jan 20, 2026

Add dodal describe that will give the docstrings of what's on the beamline #1833

Open

4 tasks

oliwenmandiamond and others added 6 commits January 20, 2026 17:19

Applied feedback

6700f93

Merge branch 'main' into device_manager_adds_device_doc_string

4791527

Removed print statement

b85c9ab

Merge branch 'device_manager_adds_device_doc_string' of ssh://github.…

f90b24a

…com/DiamondLightSource/dodal into device_manager_adds_device_doc_string

Merge branch 'main' into device_manager_adds_device_doc_string

c84146b

Fixed tests

26d3096

oliwenmandiamond requested a review from tpoliaw January 21, 2026 11:21

tpoliaw reviewed Jan 21, 2026

View reviewed changes

oliwenmandiamond and others added 3 commits January 21, 2026 13:48

Update tests/test_device_manager.py

a488bff

Co-authored-by: Peter Holloway <peter.holloway@diamond.ac.uk>

Update tests/test_device_manager.py

a3bc850

Co-authored-by: Peter Holloway <peter.holloway@diamond.ac.uk>

Applied feedback

abc3741

Simplifed test

460062e

oliwenmandiamond requested a review from tpoliaw January 21, 2026 14:18

tpoliaw reviewed Jan 21, 2026

View reviewed changes

oliwenmandiamond and others added 3 commits January 21, 2026 14:33

Merge branch 'main' into device_manager_adds_device_doc_string

2e8f50a

Added extra tests for type_docs

3817d20

Merge branch 'device_manager_adds_device_doc_string' of ssh://github.…

8afb61d

…com/DiamondLightSource/dodal into device_manager_adds_device_doc_string

oliwenmandiamond requested a review from tpoliaw January 21, 2026 14:58

Merge branch 'main' into device_manager_adds_device_doc_string

6ada650

		return _type_docs(return_type)


		def _type_docs(target: type[V1 \| V2], extra_docs: str = "") -> str:

DeviceManager adds device doc string to factory method #1829

Are you sure you want to change the base?

DeviceManager adds device doc string to factory method #1829

Uh oh!

Conversation

oliwenmandiamond commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instructions to reviewer on how to test:

Checks for reviewer

Uh oh!

DominicOram commented Jan 20, 2026

Uh oh!

oliwenmandiamond commented Jan 20, 2026

Uh oh!

dan-fernandes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oliwenmandiamond commented Jan 20, 2026 •

edited

Loading

codecov bot commented Jan 20, 2026 •

edited

Loading