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

adds pass2program and program2pass #67

Merged
merged 4 commits into from
Nov 10, 2017
Merged

adds pass2program and program2pass #67

merged 4 commits into from
Nov 10, 2017

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Oct 23, 2017

This PR provides desimodel.footprint.pass2program() and desimodel.footprint.program2pass() to provide the mapping between the integer pass numbers and string program names in the pre-defined tiles. It uses the tile definition file itself to derive the mapping instead of hardcoding which pass is which program.

This PR is a followup to desihub/desisurvey#67, which hardcoded this mapping because desimodel wasn't providing it. I'll submit a companion desisurvey PR to use this instead, while not breaking backwards compatibility.

Copy link
Member

@weaverba137 weaverba137 left a comment

Choose a reason for hiding this comment

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

The only comment I have is that program2pass() does not appear to accept vectorized input. For example, if I read a PROGRAM column from a FITS table & want to convert it to PASS, I'd have to loop over the column, rather than passing the whole column.

@sbailey
Copy link
Contributor Author

sbailey commented Oct 27, 2017

Added support for array-like input for program2pass to address review request.

@sbailey sbailey dismissed weaverba137’s stale review November 10, 2017 23:56

review requests addressed; I think this is just leftover that it hasn't been formally approved

@sbailey sbailey merged commit bef56c3 into master Nov 10, 2017
@sbailey sbailey deleted the pass2program branch November 10, 2017 23:57
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.

2 participants