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-14497: ap_pipe doesn't know filters for AssociationTask #21

Merged
merged 2 commits into from May 18, 2018

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented May 18, 2018

This PR updates ApPipeConfig to account for API changes in AssociationTask. It also adds a temporary workaround for DM-14507 (though I'm not sure why it works, since AssociationTask does the same thing internally) and improves the input validation of ApPipeConfig.

@kfindeisen kfindeisen requested a review from morriscb May 18, 2018 21:33
@@ -99,9 +99,17 @@ def setDefaults(self):
self.differencer.doSelectSources = False

self.associator.level1_db.retarget(AssociationDBSqliteTask)
# TODO: generalize in DM-12315
self.associator.level1_db.filter_names = ['u', 'g', 'r', 'i', 'z', 'y', 'VR', 'N964']
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn't using a configfile for in the CmdLineTask work here? This looks very similar to how it would happen in such a file. I'm worried that this may be adding in values specific to DECam.

Copy link
Member Author

Choose a reason for hiding this comment

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

Almost everything in the current default config is DECam-specific; DM-12315 will fix that (see also the Jira description of DM-14497).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

try:
catalog = sensorRef.get(diffType + "Diff_diaSrc")
# TODO: workaround for DM-14507
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we've found the bug in ap_association that was causing this do you want to wait for me to merge that ticket or do this for now and then remove it later?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can wait.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not fixed in DM-14517. Thanks for the help.

Currently I only include preconditions for AssociationTask;
ImageDifferenceTask's requirements are pretty unclear.
@kfindeisen kfindeisen merged commit 8482e38 into master May 18, 2018
@kfindeisen kfindeisen deleted the tickets/DM-14497 branch February 25, 2019 19:51
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