-
Notifications
You must be signed in to change notification settings - Fork 11
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
Convert UDCDirectoryProvider to standard pattern #505
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of nitty comments, but the logic and test looks good!
_directory_info = DirectoryInfo( | ||
root=directory, resource_dir=Path("panda"), prefix="", suffix="" | ||
) | ||
class UDCDirectoryProvider(UpdatingDirectoryProvider): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think change the name of this class since it should be used specifically for storing panda postition data, something like PandaPcapDirectoryProvider
?
Also, this DirectoryProvider
points to a panda
subdirectory which doesn't exist by default. We should add a comment to warn that this directory must already exist when using with a PandA - I don't think there's logic in the HDF writer or the IOC to create the directory for us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something coming down the pipe on that respects: https://github.com/bluesky/ophyd-async/blob/5c116e293afb54b437ffa2ab380635a3c96b19b2/src/ophyd_async/core/_providers.py#L116 Will be handled in the PandA hdf writer at some point. I'll revisit tomorrow
We need to create the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll add something to create the panda directory if it doesn't already exist, in a separate PR
Instructions to reviewer on how to test:
Checks for reviewer