-
Notifications
You must be signed in to change notification settings - Fork 79
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
v2: Initial support for NTNDArray and correct timestamp -> timeStamp #1148
Conversation
Seems reasonable to me, though I think we need a test. e.g. adding bits to the .db file here: https://github.com/bluesky/ophyd/blob/master/ophyd/v2/tests/test_records.db and checking that we can successfully extract the I think @coretl will have some thoughts on the dimensions of these tables, so I think we ought to wait for his input on this. |
Unsure why codecov is complaining as the lines affected haven't been touched by this PR, anyone have any ideas? |
Test added. Sorry I hadn't quite noticed how these were run previously!
From my perspective the shape isn't a blocker - I can get shape from elsewhere and reshape downstream. But long term it would be a nice-to-have for the shape to be correct here. |
Thanks for this. I'll see about adding some config for codecov to stop complaining when there are tiny changes (i.e. +/- 0.1%) to code coverage for the project/patch... |
Discussed this with @Tom-Willemsen in person and agreed that the output should be N-dimensional rather than the 1-dimensional array that is present in the PVA structure |
Closing in favor of bluesky/ophyd-async#19 now that |
Allow Ophyd V2 to read from an
NTNDArray
typepva
variable.At the moment it is using the standard array converter, which means that the array is always provided downstream as 1-D - it might be better to eventually do the reshaping into a correctly-shaped NTNDArray in the converter, but this is sufficient for my simple use-case at the moment and it wasn't entirely clear to me how to get at the metadata (i.e. shape) from within the current converter mechanism.
Also fixes a typo (?) of
timestamp
->timeStamp
, which I think would have affected every pva variable and was preventing me from reading array data over PVA. As far as I can tell from the various PVAccess specs/documents online,timeStamp
is always the standard name that is used. Happy to be corrected on this if I'm wrong though...