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}} #58

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.

Copy link
Contributor

@PaulPrice PaulPrice left a comment

Choose a reason for hiding this comment

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

Squash commits.

@@ -429,6 +429,17 @@ def __call__(self, args):
exitStatus=exitStatus,
)

def _runDataRef(self, task, dataRef, kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method needs a docstring.
I suggest naming it runTask.
I'm not a fan of the leading underscore on the name, which tells the user that this very useful method should never be overridden, which I think is a bit much to ask.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accepted and changed throughout. This makes the LegacyTaskRunner clearer, as it never called runDataRef().

@czwa czwa force-pushed the tickets/DM-2639 branch 3 times, most recently from f8b6bcb to 4d61ffd Compare August 3, 2018 22:54
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 f56c1ab into master Aug 3, 2018
@ktlim ktlim deleted the tickets/DM-2639 branch August 25, 2018 06:50
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

3 participants