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

Reporting Measure Arguments #219

Merged
merged 14 commits into from Apr 27, 2021
Merged

Reporting Measure Arguments #219

merged 14 commits into from Apr 27, 2021

Conversation

nmerket
Copy link
Member

@nmerket nmerket commented Apr 15, 2021

Fixes # . (is there an issue for this @asparke2?)

Pull Request Description

This correctly wires up the reporting measures again after the big refactor and adds the ability to specify arguments for them in the configuration file.

Checklist

Not all may apply

  • Code changes (must work)
  • Tests exercising your feature/bug fix (check coverage report on CircleCI build -> Artifacts)
  • All other unit tests passing
  • Update validation for project config yaml file changes
  • Update existing documentation
  • Run a small batch run to make sure it all works (local is fine, unless an Eagle specific feature)
  • Add to the changelog_dev.rst file and propose migration text in the pull request

@@ -146,6 +152,7 @@ def create_osw(self, sim_id, building_id, upgrade_idx):
}
])

# FIXME: Insert the reporting measures somewhere around here
Copy link
Member Author

Choose a reason for hiding this comment

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

@asparke2 I added the reporting_measures argument and validation to the commercial workflow generator, but I stopped short of implementing it because I didn't know where you wanted the measures added to your workflow.

Here's how I did it for the residential workflow. Yours will probably be very similar.

if 'reporting_measures' in workflow_args:
for reporting_measure in workflow_args['reporting_measures']:
if 'arguments' not in reporting_measure:
reporting_measure['arguments'] = {}
reporting_measure['measure_type'] = 'ReportingMeasure'
osw['steps'].insert(-1, reporting_measure) # right before ServerDirectoryCleanup

@nmerket nmerket marked this pull request as ready for review April 26, 2021 19:13
@nmerket
Copy link
Member Author

nmerket commented Apr 26, 2021

@asparke2 You feeling like this is ready? I'm thinking it is.

@nmerket nmerket assigned nmerket and unassigned asparke2 Apr 26, 2021
@nmerket nmerket requested a review from asparke2 April 26, 2021 22:31
Eagle key is optional per schema, but this code crashed if it was missing from the config file.
Copy link
Member

@asparke2 asparke2 left a comment

Choose a reason for hiding this comment

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

Looks good! I made one last commit to address a bug I just found when testing another local docker run.

@nmerket
Copy link
Member Author

nmerket commented Apr 27, 2021

Ah, good catch! I'll merge this then.

@nmerket nmerket merged commit 07fc9be into develop Apr 27, 2021
@nmerket nmerket deleted the reporting_measures branch April 27, 2021 16:57
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