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-2639: {{Standardize primary method names, run/runDataRef, across PipeTasks}} #12

Merged
merged 1 commit into from Aug 3, 2018

Conversation

czwa
Copy link
Contributor

@czwa czwa commented Jul 26, 2018

Rename methods according to RFC-352.
Fifteen total repositories with changes.

@@ -26,7 +26,7 @@ class OnlyFakesMergeTask(mBand.MeasureMergedCoaddSourcesTask):
# OnlyFakesMergeConfig to avoid having to fix overridden config parameters
ConfigClass = mBand.MeasureMergedCoaddSourcesConfig

def run(self, patchRef):
def runDataRef(self, patchRef):

Choose a reason for hiding this comment

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

I think this class also needs a run method so it can be used when you don't have a dataRef.

There are a few code style violations that are existing (e.g., i_ss, using docstrings instead of comments) that you would be welcome to clean up if you're able.

Choose a reason for hiding this comment

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

This ticket will leave some Tasks without a run method. Refactoring all the Tasks was out of scope for this ticket.

Choose a reason for hiding this comment

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

But the refactoring is not difficult, and it's required to meet the actual goal of the RFC.

Copy link
Member

@TallJimbo TallJimbo Jul 27, 2018

Choose a reason for hiding this comment

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

Sounds like we should just make sure those cleanup tickets are also conditions of calling the RFC implemented. I don't think there's anything wrong with a Task that lacks a run method (that wasn't just as wrong before) aside from the fact that this leaves the RFC implementation incomplete.

Choose a reason for hiding this comment

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

This ticket is not final implementation ticket of RFC-352: Please see https://jira.lsstcorp.org/browse/DM-11098 and the comment therein which describes the relationship between 11098 and 2639.

Prepare Tasks for Gen3 PipelineTask conversion by:
* Renaming CmdlineTask's entry method to `runDataRef`.  The default
  CmdlineTask.TaskRunner now calls a Task's `runDataRef` method on the
  parsed command line inputs.
  `runDataRef` method can take any Gen2 Butler data products.
* Renaming CmdlineTasks previous core methods (e.g. assemble,
  characterize) to `run` when they exist.
@czwa czwa merged commit 7713a09 into master Aug 3, 2018
@ktlim ktlim deleted the tickets/DM-2639 branch August 25, 2018 05:56
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

4 participants