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 CSP Bandpasses #115

Merged
merged 7 commits into from
Nov 9, 2015
Merged

Add CSP Bandpasses #115

merged 7 commits into from
Nov 9, 2015

Conversation

dannygoldstein
Copy link
Contributor

@kbarbary
Copy link
Member

kbarbary commented Nov 8, 2015

Sweet. A couple notes:

  • All the existing built-in bandpass names are all lowercase. Can you change, e.g., cspB to cspb to follow this convention?
  • Remove extra spaces after , in builtins.py to follow PEP8 and get tests passing.
  • csphs / csphd for denoting Swope and Dupont is OK but seems pretty cryptic (even more so than band names normally are). On the other hand csphswope /csphdupont gets hard to read. I've boxed us in here by not using _ in any existing bandpasses! If there's no good ideas we can stick with the extremely short names.

@dannygoldstein
Copy link
Contributor Author

First and third bullets are done. For the third, I added a column to csp_filter_info.dat that indicates the telescopes (Swope or DuPont) to which the filters are attached. The second bullet was OK as of e7a62f8.

All unit tests are passing.

@dannygoldstein
Copy link
Contributor Author

Hold on. There is a typo on the CSP website. On http://csp.obs.carnegiescience.edu/data/filters, the second V band filter is called "V-3099", whereas in Stritzinger et al. (2011), it's called "V-3009". Updating this here.

@dannygoldstein
Copy link
Contributor Author

This is ready to go.

@kbarbary
Copy link
Member

kbarbary commented Nov 9, 2015

OK, sounds good to me.

kbarbary added a commit that referenced this pull request Nov 9, 2015
@kbarbary kbarbary merged commit e4e254a into sncosmo:master Nov 9, 2015
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