RadioAstronomySoftwareGroup / RadioAstronomySoftwareGroup/pyuvdata

SImplifying read_cst_beam

Open
#1,236 0 comments 1 reaction 2 assignees View on GitHub

@bhazelton is already working on this.

Since Dec 8, 2022.

beams technical debt
Dominant language
Python
Stars
102
Forks
35
Avg merge
1d 20h
Merged PRs (30d)
14

Description

Having a look through UVData.read_cst_beam I get the impression it could be MUCH simplified. Some things I noticed:

  1. There is a discrepancy between some of the keywords to the function, and the keys expected in the YAML file (eg. "frequency/frequencies" and "reference_impedance/ref_imp").
  2. Exactly the same huge set of parameters is used, copy-pasted 3 times in the function. At the very least, these should be packed up into a dict and unpacked on each call.
  3. There's a bunch of weird handling of over-riding YAML inputs with direct inputs to the function. I think it's debatable whether this should be allowed at all (isn't that allowing bad practice?), but if we do allow it, it would be much much easier to take in the parameters of the function as an explicit dictionary, and just do a dict update (point 1 would have to be fixed, of course). This would remove at least 3 repetitions of naming every single parameter that I can see.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.