RadioAstronomySoftwareGroup / RadioAstronomySoftwareGroup/pyuvdata
SImplifying read_cst_beam
Open
@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:
- 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").
- 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.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.