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-23008: Add a script to define DCR subfilters in a Gen 3 registry. #338

Merged
merged 1 commit into from Jan 31, 2020

Conversation

isullivan
Copy link
Contributor

No description provided.

"""

def __init__(self, *, config=None, **kwargs):
super().__init__(config=config, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this init if you are doing nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm afraid I was just following what you did in MakeGen3SkyMapTask.
I assumed there was a reason we needed to strip positional arguments from __init__, but if that's not needed I'm happy to remove this.

Copy link
Contributor

Choose a reason for hiding this comment

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

It may have been put in as a safeguard since Task takes config as a positional argument, and this ensures that happens correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I think I will leave it in, then, but I'll add a comment.

# Build a parser for command line arguments
parser = argparse.ArgumentParser(description="Define the set of subfilters for chromatic modeling.")
parser.add_argument("butler", metavar="Butler", type=str, help="Path to a gen3 butler")
parser.add_argument("collection", type=str, metavar="Collection",
Copy link
Contributor

Choose a reason for hiding this comment

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

follow DM-21246 that is going into review now-ish, and that will remove the need for the collection argument to create a butler, since what you are doing is not dependent on a butler at all.

)


class MakeGen3DcrSubfiltersTask(pipeBase.Task):
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you be running this command multiple times for multiple filters to add subbands for them all? Then I think it might be worth it to use ListFields in the config and do them all at once here. At the very least it gives you the option to do it all at once, and still have a single entry if that is all you need.

@isullivan isullivan force-pushed the tickets/DM-23008 branch 2 times, most recently from adf1306 to c9f076c Compare January 30, 2020 20:33
@isullivan isullivan merged commit 49bd8b9 into master Jan 31, 2020
@timj timj deleted the tickets/DM-23008 branch February 18, 2021 15:49
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