Updating custom_financial configuration to use subdivided dictionary form #285
Conversation
This file contains 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
bayc
added
enhancement
…ub.com/bayc/HOPP-1 into feature/add_config_to_custom_financial
…add_config_to_custom_financial
…add_config_to_custom_financial
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
NOTE: Similar capabilities were achieved in #289. This PR was not merged due to issues with tests failing, most likely due to unintended effects within the custom financial model.
Expand
custom_financial_modelto use a more well-defined dictionary and utilize defaults for thedefault_fin_configThis PR updates
custom_financial_modelto use a dictionary that separates the configuration dictionary by technology. It also established default dictionaries within the code so the user does not have to supply thedefault_fin_config.yamlexplicitly. This simplifies setup and more easily enables a user to update specific parts of the default financial configuration for different technologies.Related issue
#284
Impacted areas of the software
Mostly
custom_financial_model.pySmall updates in each of the technologies
Tests
Additional supporting information
This method still requires the user to specify a dictionary to use the custom financial model. In the inputs files, where we don't want to specify the input explicitly or point to a yaml file, I have included an empty dictionary. This could be changed if we updated the logic in each of the technologies for when the custom financial model is used. I think it would be good to update this to use a flag, but is worth more discussion.
Test results, if applicable
Tests are not passing.