Skip to content
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

mpol.utils.convert_baselines and mpol.utils.broadcast_and_convert_baselines duplicate visread functionality #227

Closed
iancze opened this issue Dec 6, 2023 · 1 comment · Fixed by #248

Comments

@iancze
Copy link
Collaborator

iancze commented Dec 6, 2023

Is your feature request related to a problem or opportunity? Please describe.
The two utility routines mpol.utils.convert_baselines and mpol.utils.broadcast_and_convert_baselines duplicate functionality that is provided by visread. This creates another source of code to maintain. IIRC, we originally duplicated this into MPoL this because a) we want to store visibility datasets using baselines measured in meters because if nchan > 1 it is a more efficient way to store them than lambda (all baselines are the same in meters, but lambda values change when observing frequency changes) and b) we don't want to require visread as a dependency of MPoL because as it's currently implemented, visread has a core dependency of casatools, which significantly restricts the ability to install MPoL into modern compute environments. However...

Describe the solution you'd like
... I think a better solution is to reorganize visread into a set of minimal functionality (including these aforementioned routines) that does not require casatools. The larger set of visread functionality can be maintained through a pip install visread[casa] install, and cleanly separated into their own module imports. It may or may not make sense to add the minimal visread to the core set of MPoL dependencies, since this is not really core MPoL functionality, it's more of a data prep helper.

How might you be able to help with the development of this feature?
I'll do this to visread.

@iancze
Copy link
Collaborator Author

iancze commented Dec 7, 2023

I've now completed the reorganization of visread via visread #9. So the next steps here are to remove the mpol.utils.convert_baselines and mpol.utils.broadcast_and_convert_baselines routines from MPoL and change any tutorials or other scripts to use the visread.process.convert_baselines and visread.process.broadcast_and_convert routines.

Also move fourier.safe_baseline_constant_kilolambda and fourier.safe_baseline_constant_meters to visread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant