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-20570: Pipeline failure treated as ap_verify success #96

Merged
merged 2 commits into from Aug 9, 2019

Conversation

kfindeisen
Copy link
Member

This PR fixes some incorrect (out of date?) documentation of what CmdLineTask.parseAndRun returns for a typical task runner.

@kfindeisen kfindeisen requested a review from mrawls August 2, 2019 21:35
python/lsst/pipe/base/cmdLineTask.py Outdated Show resolved Hide resolved
@ktlim
Copy link
Contributor

ktlim commented Aug 7, 2019

Can someone see if this is compatible with the changes in DM-20376 that I haven't had a chance to complete, test, and merge?

@timj
Copy link
Member

timj commented Aug 7, 2019

@ktlim I've made PR #97 to make it easier to see what DM-20376 is trying to do.

@jonathansick
Copy link
Member

The new style guideline for documenting structs is a definition list: https://developer.lsst.io/python/numpydoc.html#struct-types

@kfindeisen
Copy link
Member Author

@ktlim This documentation (CmdLineTask.parseAndRun) is not changed in #97, but should be brought in sync with its changes to TaskRunner.run.

I'm happy to close this PR and let #97 take care of things, especially since it looks like I'll need to guard against None values after all...

@kfindeisen
Copy link
Member Author

@ktlim should I merge my changes to CmdLineTask.parseAndRun (with the requested corrections), then let #97 edit the text for its changes, or will #97 make all the necessary changes to make the parseAndRun docs up-to-date?

@ktlim
Copy link
Contributor

ktlim commented Aug 9, 2019

Go ahead and merge this.

@kfindeisen kfindeisen merged commit dca528c into master Aug 9, 2019
@kfindeisen kfindeisen deleted the tickets/DM-20570 branch August 9, 2019 20:01
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

5 participants