pyuvdata
pyuvdata copied to clipboard
Telescope frame
Description
Introduces a new parameter telescope_frame
to UVData
, which can be either "ITRS" (default) or "MCMF" (Moon-Centered Moon-Fixed). This new parameter indicates the coordinate frame the telescope_position
is defined in. Several utility functions and UVData methods are configured to use this parameter:
- ENU_from_ECEF
- ECEF_from_ENU
- LatLonAlt_from_XYZ
- XYZ_from_LatLonAlt
- get_lst_for_time
The package lunarsky
is needed to use the MCMF frame. It's been added as an optional dependency.
Motivation and Context
pyuvsim
relies on pyuvdata
for computing uvw coordinates and correctly transforming antenna positions. While pyradiosky
and pyuvsim
both support lunarsky
coordinate frame definitions, so far pyuvdata
has not included this. Once this is merged, a corresponding small change to pyuvsim
should fix https://github.com/RadioAstronomySoftwareGroup/pyuvsim/issues/408.
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation change (documentation changes only)
- [ ] Version change
- [ ] Build or continuous integration change
Checklist:
- [x] I have read the contribution guide.
- [x] My code follows the code style of this project.
New feature checklist:
- [x] I have added or updated the docstrings associated with my feature using the numpy docstring format.
- [ ] I have updated the tutorial to highlight my new feature (if appropriate).
- [x] I have added tests to cover my new feature.
- [ ] All new and existing tests pass.
- [ ] I have updated the CHANGELOG.
Codecov Report
Merging #1199 (60f6f7d) into main (183597b) will increase coverage by
0.00%
. The diff coverage is100.00%
.
Additional details and impacted files
@@ Coverage Diff @@
## main #1199 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 32 32
Lines 17405 17472 +67
=======================================
+ Hits 17389 17456 +67
Misses 16 16
Impacted Files | Coverage Δ | |
---|---|---|
pyuvdata/utils.py | 100.00% <100.00%> (ø) |
|
pyuvdata/uvdata/uvdata.py | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 183597b...60f6f7d. Read the comment docs.
I would think that the LST should be set to the RA at zenith.
@bhazelton My changes to the tests should cover the new cases, but I'm not sure how to change the circleci config to install lunarsky
and run them.
@aelanman sorry for the delay. I just added lunarsky
to some of the test environments. It looks like there's now only one uncovered line. I need to make some more updates for it but I needed to know if there was a minimum version of lunarsky to use? Also, it looks like you need to rebase the branch.
@bhazelton Thanks for fixing that! I've made some changes to cover the remaining missing lines, and rebased off main
. For lunarsky, I would recommend a minimum version of 0.1.2
, which is the third release.
@aelanman we discussed this in last week's telecon and we're broadly supportive but we don't really like the approach of replacing the astropy SkyCoord with the lunarsky one if lunarsky is installed. We'd prefer to import the lunarsky one as a different name and only use it if needed (i.e. if the telescope is on the moon). Similarly we'd like to avoid using the mock classes in case lunarsky is not installed. I'm happy to tackle these changes if you're too busy on other projects.
@bhazelton I've made some changes which ensure SkyCoord and Time are imported with separate names for astropy and lunarsky, and the lunarsky versions (LSkyCoord and LTime) are only used when a lunarsky-related class is used. It also removes the dummy classes. Fortunately, the changes here used lunarsky sparingly and mostly defined new XYZ/LatLonAlt transformations by using basic erfa functions. There was a slight hit in coverage; I'll have to fix that later.
The main reason for replacing the class globally is that it can be hard to predict where each form will be needed. The lunarsky classes inherit from their astropy equivalents, but just add support for the new MoonLocation
class, etc. In principle, they should be identical (including passing tests of "isinstance"). However, it can be confusing if you look at the, say, "SkyCoord" instance and its class is lunarsky.sky_coord.SkyCoord
.