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

DM-25040: ap_association uses physical filter in Gen 3 #83

Merged
merged 2 commits into from May 27, 2020

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented May 22, 2020

This PR fixes a bug that caused MapDiaSourceTask to sometimes report observatory-specific filters, which would then get propagated to table columns. It updates all ap_association code to standardize filter names.

Previously, the physical filter would get used if that's how it was
stored in the exposure, causing plugins to generate nonstandard
columns that broke the database.
@@ -805,7 +805,7 @@ def get_ccd_visit_info_from_exposure(exposure):
# flux zero point error ``counts``]
values = {'ccdVisitId': visit_info.getExposureId(),
'ccdNum': exposure.getDetector().getId(),
'filterName': filter_obj.getName(),
'filterName': filter_obj.getCanonicalName(), # use abstract filter
Copy link
Member Author

Choose a reason for hiding this comment

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

get_ccd_visit_info_from_exposure doesn't seem to be used anywhere; can it be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes it can. It's left over from a previous iteration of the Apdb and, as you point out, is not used anywhere.

@kfindeisen kfindeisen requested a review from morriscb May 22, 2020 23:23
@kfindeisen kfindeisen merged commit b996882 into master May 27, 2020
@kfindeisen kfindeisen deleted the tickets/DM-25040 branch May 27, 2020 21:02
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