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

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

@r-owen r-owen left a comment

Choose a reason for hiding this comment

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

I suggest adding run to measureCrosstalk.py

@@ -195,7 +195,7 @@ def parseAndRun(cls, *args, **kwargs):
pickle.dump(resultList, open(results.parsedCmd.dumpRatios, "w"))
return task.reduce(resultList)

def run(self, dataRef):
def runDataRef(self, dataRef):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a case where runDataRef should call run since run can do useful work on the unpersisted exposure.

Copy link
Contributor

Choose a reason for hiding this comment

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

The work of converting tasks to have the work done in run, and the runDataRef be the entry point for command line task is planned for after this ticket is complete. We chose to do the big outward facing api change first, and save any refactoring work for another ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a trivial change. Most of the other renames do not lend themselves to adding run methods (at least that I saw in the packages I looked at) but this one did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the interest in merging the changes for this ticket that refactoring depends on, @yalsayyad has helped by opening a ticket on this refactorization. DM-15311 covers this change, and I'll begin work on it next week.

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 b1634db into master Aug 3, 2018
@ktlim ktlim deleted the tickets/DM-2639 branch August 25, 2018 06:44
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