Refactor the telescope-related metadata in UVData, UVCal and UVFlag into an attached Telescope object

[bhazelton]

trafficstars

Description

All the metadata which is really about the telescope (rather than the dataset) has been pulled out into a Telescope object which is then attached to each object as an attribute. I used __getattr__ and __setattr__ on UVBase to keep the old API (but with deprecation warnings). I think other the biggest difference a user might notice is that some new metadata can show up on UVCal and UVFlag (e.g. instrument and antenna_diameters).

This really streamlines a bunch of code and will make it easier to maintain.

Motivation and Context

closes #1095

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:

• [ ] 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.
• [x] All new and existing tests pass.
• [ ] I have updated the CHANGELOG.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.93%. Comparing base (b54fce4) to head (f52fc0a).

Additional details and impacted files

@@             Coverage Diff             @@
##           prep_v3.0    #1422    +/-   ##
===========================================
  Coverage      99.92%   99.93%            
===========================================
  Files             41       41            
  Lines          22406    22702   +296     
===========================================
+ Hits           22390    22687   +297     
+ Misses            16       15     -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[bhazelton]

@steven-murray I did add a new property on the Telescope object called location_obj which returns an EarthLocation (or a MoonLocation) so that a user can always just get the object. But I can look at making the location just be such an object, that might clean some things up.

I like the HDF5 idea, I had thought about doing something like that myself. I'll work on it.

[bhazelton]

@steven-murray @kartographer I think this ready for another look when you have time. I think I've addressed all your comments and what we discussed in the telecons.

[bhazelton]

Astropy 6.1 dropped on pypi (but interestingly not yet on conda) and a new warning is being generated:

FutureWarning: Setting the location attribute post initialization will be disallowed in a future version of Astropy. Instead you should set the location when creating the Time object. In the future, this will raise an AttributeError.

I fixed a few places where we were causing this in pyuvdata, but most of them are coming from lunarsky, so I made an issue there (https://github.com/aelanman/lunarsky/issues/27).

This is causing our warnings tests to fail. Not sure if I want to limit astropy on that vs just waiting for it to be fixed on lunarsky.

[bhazelton]

@bhazelton looks good overall -- I don't have any detailed comments beyond what @steven-murray has already marked, the only thing that I think would be worth discussing is what will happen when we have new parameters to add to the Telescope object when reading/writing to HDF5-formatted files (this is of particular interest for SMA, since I've got some parameters already in mind that I'll start working on once v3 is done). Right now, everything is getting written to the main header, and I don't know if that's what we want to keep on doing, or if we want to have the new parameters grouped in some sensible way. I did this in an ad-hoc fashion w/ phase_center_catalog, which of course worked okay but changed shortly after that was integrated, and so if we don't want to trust my on-the-fly approach (which I never do 😉) it might be good to discuss how we might do this in the future and make some comments in the code accordingly.

Good question. I'd really like this to be a larger conversation as it pertains to the UVH5 format which is being used beyond pyuvdata at this point. Currently in that format we really only differentiate between header and data items, where anything that's not the size of visibility data are lumped in the header. The phase center catalog is also in header, although it is a nested dataset. On the one hand, I understand the niceness of grouping telescope related data within a dataset, especially as we are now representing it that way internally to pyuvdata, but on the other hand since the format is more widely used I don't know that we should force our object hierarchy on the format too strictly. Maybe @plaplant or @david-macmahon have thoughts?

[steven-murray]

To add my two cents to some other issues brought up here (which we should move away into separate discussions...)...

1. I prefer the idea of a well-formed uvh5 file format where the telescope metadata is in its own sub-group that can be read completely independently. This would mean you could literally have hdf5 files with only telescope metadata in them and they would make sense, and could be read directly as Telescope objects. It's just more flexible. However, the old uvh5 file format should be supported forever. That is, there should be legacy UVH5 readers that know how to read older formats, and can read them perfectly fine as uvdata objects. My preferred way of doing this is to have separate reading classes/functions for different versions of the file format, and a dispatcher that knows which function to dispatch to given the file contents. I don't think we should have to worry about whether people are using older formats... the pyuvdata code itself should be how the files are read, and anyway each version of the file format should be documented, so people can use any format.
2. The movement of astropy away from setting attributes after object creation is unsurprising to me, and is how I (and I think most people) expect python objects to work. This is in contrast to pyuvdata, which is jarring to me every time I use it for this reason. It might be worth thinking about (for v4...) refactoring towards this modality.