astrocatalogs / astrocats

Astrocats package for constructing astronomical catalogs
https://astrocats.space
MIT License
40 stars 12 forks source link

`add_*` functions in `test` should use `KeyCollection` members rather than literals #61

Open guillochon opened 8 years ago

guillochon commented 8 years ago

So currently when we do something like say add_photometry, we pass a list of kwargs to the function where each kwarg is really a string:

catalog.entries[name].add_photometry(time=mjd, band=band, magnitude=magnitude, 
    e_magnitude=e_magnitude, source=source)

But, since say magnitude could be changed to a different string later, it might be better to pass the kwargs as a dictionary, with the dictionary members pulling from the KeyCollection members, like so:

catalog.entries[name].add_photometry(**{PHOTOMETRY.TIME: mjd, PHOTOMETRY.BAND: band, 
    PHOTOMETRY.MAGNITUDE: magnitude, PHOTOMETRY.E_MAGNITUDE: e_magnitude, 
    PHOTOMETRY.SOURCE: source})

This makes the add_* calls a bit longer but also more stable to future changes. Should we adopt this as a best practice?

lzkelley commented 8 years ago

An impassioned definitely. And for future tasks, that should actually make adding data a lot smoother/cleaner (e.g. more templated --- like what I'm trying to do here).

guillochon commented 8 years ago

We should do this for the test task in astrocats; then I would consider this issue closed at least for the main repo. Will take time to do this for supernovae, etc., but those should be separate issues for those modules.