Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support lazy devices #515

Closed
wants to merge 6 commits into from
Closed

Conversation

stan-dot
Copy link
Contributor

Outlined the places where the code needs to be modify to support the requested features.

Some alignment must happen on what is the best format to store the devices that are lazy.

I'd rather do it in the start of the PR.
#363 and #504 would change how this works however I think that this PR is mergeable much earlier than that.

Then, on my end I think the lazy devices would be best presented as:

  • early_devices: dict[str, Device] = field(default_factory=dict)
  • lazy_devices: dict[str, Device] = field(default_factory=dict)
    properties on the Context object.

@stan-dot stan-dot added enhancement New feature or request python Pull requests that update Python code spike Issue to investigate scope/feasilibilty/technology etc. of how to do something labels Jun 20, 2024
@stan-dot stan-dot self-assigned this Jun 20, 2024
@stan-dot stan-dot linked an issue Jun 20, 2024 that may be closed by this pull request
Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.06%. Comparing base (53bdbe2) to head (057367b).

Current head 057367b differs from pull request most recent head 8ed8301

Please upload reports for the commit 8ed8301 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #515      +/-   ##
==========================================
- Coverage   91.10%   91.06%   -0.05%     
==========================================
  Files          40       40              
  Lines        1787     1779       -8     
==========================================
- Hits         1628     1620       -8     
  Misses        159      159              

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

@callumforrester
Copy link
Contributor

PR title

@stan-dot stan-dot changed the title added the outline of the PR. need to discuss the caching format Support lazy devices Jun 21, 2024
@stan-dot stan-dot force-pushed the 440-support-parallel-and-lazy-connect branch 3 times, most recently from 83208ca to 6ca50dd Compare June 26, 2024 16:04
src/blueapi/core/context.py Show resolved Hide resolved
src/blueapi/core/context.py Outdated Show resolved Hide resolved
@stan-dot
Copy link
Contributor Author

Todo: just fix tests, then push and mark PR as ready

@stan-dot
Copy link
Contributor Author

I commented in the ophyd-async repo towards the resolution of the test errors here

@coretl
Copy link
Contributor

coretl commented Jun 28, 2024

The relevant ophyd-async ticket is bluesky/ophyd-async#387, not bluesky/ophyd-async#413.

The short term fix is to cap numpy<2.0.0 like bluesky/ophyd-async#388.

@stan-dot stan-dot force-pushed the 440-support-parallel-and-lazy-connect branch from c6e9ace to ce7ec31 Compare June 28, 2024 12:17
@stan-dot
Copy link
Contributor Author

stan-dot commented Jul 1, 2024

@callumforrester so we need to support both ophydv1 and ophyd-async devices?

@stan-dot
Copy link
Contributor Author

stan-dot commented Jul 1, 2024

image
@callumforrester why do we import this as a sum of interfaces? I thought this kind of thing already exists inside bluesky?

@stan-dot stan-dot force-pushed the 440-support-parallel-and-lazy-connect branch from 420f27f to 8ed8301 Compare July 2, 2024 07:24
@stan-dot
Copy link
Contributor Author

stan-dot commented Jul 2, 2024

I just realized that there is no clear way to see if a device is lazy or not. this might need to wait until DiamondLightSource/dodal#483 is done

@callumforrester
Copy link
Contributor

@stan-dot yes both should be supported for now, dodal is going to fully move off ophys v1 eventually, but probably not soon

@stan-dot
Copy link
Contributor Author

stan-dot commented Aug 8, 2024

closing as out of date - the change of blueapi plan to register_plan is unrelated. furthermore no PR is meaningful here until the dodal issue is done

DiamondLightSource/dodal#483

@stan-dot stan-dot closed this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Pull requests that update Python code spike Issue to investigate scope/feasilibilty/technology etc. of how to do something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support parallel connect
4 participants