-
Notifications
You must be signed in to change notification settings - Fork 0
Formalise Measures Handling #138
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
Conversation
|
@JSKenyon If you want to take a look, I'd appreciate it. The main takeaway is that MSv4 measures attributes can now be extracted seamlessly from underlying the MEASINFO in the CASA column keywords, rather than assuming a specific type of measures and the associated hard-coding. Don't feel obliged though and I may just merge. |
… to resolve Measures data
|
In
Possibly this could be reworked with a Facade |
|
Thanks for the review request; no need to wait on me but I will take a look a little later. |
JSKenyon
left a comment
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.
Not that my approval is needed but looks good to me! Left one comment about a docstring.
|
Thanks for the review. |
Currently, the measures code for both the MAIN table and sub-table targets specific columns when extracting measures data from the MSv2 dataset.
For e.g.
MAIN::TIME,SPECTRAL_WINDOW::CHAN_FREQandSPECTRAL_WINDOW::REF_FREQUENCY, orANTENNA::POSITIONScolumns are hard-coded to haveepoch,frequencyandlocationmeasures handlers associated with them.This can be improved in the case of subtables as they are loaded into memory as arrow Tables and the
VarRefCol, if present for a target column, can be read directly from this table.This PR reworks the measures handling code to automatically derive a Measures coder in the case of an arrow Table. The MAIN table columns for which it is assumed
VarRefColmeasures referenceswill still need to be somewhat manually specified
Thanks for contributing to xarray-ms.
We would appreciate it if you could add:
doc/source/changelog.rst.