Adjust to new DirectoryProvider structure #376
Conversation
|
Requires bluesky/ophyd-async#86 and a release of ophyd-async |
DiamondJoseph
force-pushed
the
directoryInfo
branch
from
70d2071
to
0bc8e90
Compare
|
Noting that this will need to be moved: #353 |
| ) | ||
| directory_info: DirectoryInfo = provider() | ||
| yield from bpp.inject_md_wrapper(plan, md={DATA_SESSION: directory_info.prefix}) | ||
| provider.clear() |
There was a problem hiding this comment.
Won't this mean that detectors subsequently calling provider() will try to write to None?
There was a problem hiding this comment.
Yes, I think the issue is actually that update() should always fetch a new instance rather than only the first time.
There was a problem hiding this comment.
It was intended to be a caching thing, where update() would generate a brand new scan number and save location, so you would end up with this:
If it fetches a new instance every time, wouldn't the detectors each write to a different scan number, which would be different in turn from the resource doc?
There was a problem hiding this comment.
Yes, you're right (and update() doesn't hold onto the previous one as I thought it did in the previous comment), I was confused by the change to no longer swallow the Exception in update() [as required merge into Bluesky had occured], which previously made sure the prior collection was None but no longer does! ⚡
DiamondJoseph
force-pushed
the
directoryInfo
branch
2 times, most recently
from
429df84
to
5ce6ac2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #376 +/- ##
==========================================
+ Coverage 89.96% 90.16% +0.19%
==========================================
Files 43 41 -2
Lines 1814 1708 -106
==========================================
- Hits 1632 1540 -92
+ Misses 182 168 -14 ☔ View full report in Codecov by Sentry. |
|
IMPORTANT: Must also now unpin dependency on dodal |
DiamondJoseph
force-pushed
the
directoryInfo
branch
from
f6e8692
to
3dd6fa9
Compare
DiamondJoseph
force-pushed
the
directoryInfo
branch
2 times, most recently
from
950d315
to
9ce79fd
Compare
callumforrester
force-pushed
the
directoryInfo
branch
from
af8d1dd
to
80f61c6
Compare
There was a problem hiding this comment.
As it is this is fine, all the code seems to be replaced in DiamondLightSource/dodal#445. Except there are tests in this PR that have no correspondent in dodal. Could you make a PR on dodal that moves the appropriate tests over?
I've been working on it today: the FakeDetector structure wasn't correct for how a StandardDetector actually uses a DirectoryProvider |
DiamondJoseph
dismissed
DominicOram’s stale review
Tests have been merged in dodal now
Allows us to unpin our dependency on dodal (through it, ophyd-async)