Merged
Conversation
be95b1a to
867f950
Compare
867f950 to
d4d2312
Compare
Merged
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
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.
Purpose
Simplify the getting of collaborative scaling next capacities. Once targets & resources are defined,
collaborative_strategy_manager.data_frame_of_next_capacities()seems to be the top-level user-facing function, but there's no input forsolar_fractionthere. We could add an input here, pass that input tocalculate_capacity_scaling(), and then pass that input tocalculate_total_added_capacity(), or we could add one call tocollaborative_strategy_manager.set_solar_fraction()before callingdata_frame_of_next_capacities()and then we don't have to worry about propagating an input three levels deep.What is the code doing
solar_fractionattribute to__init__(), defaulting toNone._check_solar_capacity()to check inputs.set_solar_fraction()which assigns values to the instance attribute.calculate_total_added_capacity()to not take an input forsolar_fraction, but to get it from the instance attribute instead._check_solar_capacity()to replace functionality inTargetManager.__init__()Validation
The code has been tested, and using the setter results in changes to the output capacities. The setter complains on bad inputs. All existing unit tests still pass.
Time to review
Five to fifteen minutes. This feature is not urgent, since I've already used the code for what I need it for right now, and I can always rebase and re-use it in the meantime. It would be nice to have in the codebase eventually though.