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

Review for DM-2837 #15

Merged
merged 1 commit into from Jun 13, 2016
Merged

Review for DM-2837 #15

merged 1 commit into from Jun 13, 2016

Conversation

pschella
Copy link

@pschella pschella commented Jun 7, 2016

This commit adds a simple unittest to check that colorterms for
CFHT override correctly. The unittest ensures that

  1. The colorterms provided in config/colorterms.py are formatted
    correctly, and
  2. The actual values read in match the values in config/colorterms.py
    for a given filter.

@pschella pschella changed the title Add unittest for colorterms override. Review for DM-2837 Jun 7, 2016
def testColorterms(self):
"""Test that the colorterm libraries are formatted correctly"""
refBands = ["u", "g", "r", "i", "z"]
cfhtBands = ["u","g","r","i","z"]
Copy link
Author

@pschella pschella Jun 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to be consistent with spacing (I, and some PEP8 tools, prefer space after comma in sequences).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why have both refBands and cfhtBands? If both are needed (because one is modified later), and if both are supposed to be equal, why not make this clear with cfhtBands = refBands[:]

@AstroVPK AstroVPK force-pushed the tickets/DM-2837 branch 6 times, most recently from f65b0ee to 5fdf73d Compare June 13, 2016 15:50
This commit adds a simple unittest to check that colorterms for
CFHT override correctly. The unittest ensures that
1. The colorterms provided in config/colorterms.py are formatted
   correctly, and
2. The actual values read in match the values in config/colorterms.py
   for a given filter.
@AstroVPK AstroVPK merged commit 01f50ea into master Jun 13, 2016
@ktlim ktlim deleted the tickets/DM-2837 branch August 25, 2018 05:50
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 this pull request may close these issues.

None yet

2 participants