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

Tickets/dm 11300 #46

Merged
merged 5 commits into from Jul 18, 2017
Merged

Tickets/dm 11300 #46

merged 5 commits into from Jul 18, 2017

Conversation

RobertLuptonTheGood
Copy link

No description provided.

wmwv added 4 commits July 17, 2017 22:14
This fixes the literal error, but I still need to fix the
logical error in run, where outputPrefix isn't calculated until later.
so that it's available to all runOne* functions run in there.
if outputPrefix is not None:
thisOutputPrefix = "%s_%s_" % (outputPrefix.rstrip('_'), filterName)
else:
thisOutputPrefix = "%s" % filterName

Choose a reason for hiding this comment

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

I'd invert the test -- avoid double negatives where possible

@@ -137,6 +145,7 @@ def run(repo_or_json, metrics=None, makePrint=True, makePlot=True,
if not os.path.isdir(repo_or_json):
print("Could not find repo %s" % (repo_or_json))
return
kwargs['metrics'] = metrics

Choose a reason for hiding this comment

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

Is this part of this ticket?

Copy link
Contributor

Choose a reason for hiding this comment

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

Under a slight expansion of the ticket to be "fix things that MWV broke in DM-11215 so that things run again", then yes. I've expanded the ticket description to include this.

The general thing I was doing wrong was not correctly tracking which arguments were being passed through **kwargs but sometimes still needed to be inspected by the intervening functions, or which were being passed as explicit keywords to a function but not being passed along to functions called.

[Yes, this suggests some better design work is called for. I'm somewhat saving that for adapting validate_drp into a true CommandLineTask DM-5096.]

@wmwv wmwv merged commit a707dfd into master Jul 18, 2017
@ktlim ktlim deleted the tickets/DM-11300 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

2 participants