Skip to content
This repository was archived by the owner on Sep 2, 2024. It is now read-only.

Use callbacks in grid detection plan#731

Merged
DominicOram merged 12 commits into
mainfrom
671_callbacks_in_grid_detection
Jun 16, 2023
Merged

Use callbacks in grid detection plan#731
DominicOram merged 12 commits into
mainfrom
671_callbacks_in_grid_detection

Conversation

@noemifrisina

Copy link
Copy Markdown
Contributor

Fixes #671

Link to dodal PR (if required): #91
(remember to update setup.cfg with the dodal commit tag if you need it for tests to pass!)

To test:

1.Run tests

@codecov

codecov Bot commented Jun 13, 2023

Copy link
Copy Markdown

Codecov Report

Merging #731 (d6610ed) into main (08e6505) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #731      +/-   ##
==========================================
+ Coverage   93.95%   94.01%   +0.05%     
==========================================
  Files          35       36       +1     
  Lines        1721     1737      +16     
==========================================
+ Hits         1617     1633      +16     
  Misses        104      104              
Impacted Files Coverage Δ
src/artemis/experiment_plans/full_grid_scan.py 100.00% <100.00%> (ø)
...rtemis/experiment_plans/oav_grid_detection_plan.py 100.00% <100.00%> (ø)
...nal_interaction/callbacks/oav_snapshot_callback.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor comments in code, I think we should also merge #730 first so that we can ensure it doesn't break tests

oav_callback = OavSnapshotCallback()
yield from bps.subscribe(oav_callback)

yield from grid_detection_plan(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should: I think you can do the subscribe/unsubscribe in a decorator

path_join(snapshot_dir, f"{snapshot_filename}.png"),
]
)
yield from bps.create("snapshot_directories")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should: This isn't just directories any more

@DominicOram

Copy link
Copy Markdown
Collaborator

Spun out #733 and #734

@noemifrisina

Copy link
Copy Markdown
Contributor Author

Done, thanks

@noemifrisina
noemifrisina requested a review from DominicOram June 14, 2023 11:56
@DominicOram

Copy link
Copy Markdown
Collaborator

Could you:

@DominicOram DominicOram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks

@DominicOram
DominicOram merged commit 2653769 into main Jun 16, 2023
@DominicOram
DominicOram deleted the 671_callbacks_in_grid_detection branch June 16, 2023 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use callbacks in grid detection

2 participants