Validator improvements#350
Merged
Merged
Conversation
zolanaj
approved these changes
Aug 19, 2022
Collaborator
zolanaj
left a comment
zolanaj
left a comment
There was a problem hiding this comment.
@hdunham Thanks, this looks great! I added the following tests to yours:
- Assert that the starting post doesn't trigger any missing required input errors, with
post["Settings"]["off_grid_flag"]set to True and False (might be overkill, but could be helpful if we add separate required inputs to either ongrid or offgrid in the future) - When
post["Settings"]["off_grid_flag"] = True, assert that"ElectricTariff"inputs do not show up in the validation errors, even if missing
If you agree with this, please go ahead and merge. Thanks again!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Bug fixes
What is the current behavior?
ElectricTariff key is never required, even for on-grid
ElectricUtility model not created if not provided in inputs of an on-grid analysis
ElectricUtility silently ignored if provided in inputs for an off-grid analysis
What is the new behavior (if this is a feature change)?
Require ElectricTariff key for on-grid
Create and save ElectricUtility model if not provided in inputs of an on-grid analysis, in order to have the default inputs
Warn that ignoring ElectricUtility if provided in inputs for an off-grid analysis
Add test to ensure errors returned when required keys are missing
Does this PR introduce a breaking change?
no
Other information: