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

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

Open
guillochon opened this issue Jul 17, 2016 · 2 comments
Assignees
Milestone

Comments

@guillochon
Copy link
Member

guillochon commented Jul 17, 2016

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
Copy link
Member

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
Copy link
Member Author

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.

@guillochon guillochon changed the title Should add_* functions also use KeyCollection members rather than literals? add_* functions in test should use KeyCollection members rather than literals Jul 27, 2016
@guillochon guillochon added this to the v0.3 milestone Jul 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants