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

refactor: set grid_model property on grid #494

Merged
merged 3 commits into from
Jun 3, 2021
Merged

refactor: set grid_model property on grid #494

merged 3 commits into from
Jun 3, 2021

Conversation

jenhagg
Copy link
Collaborator

@jenhagg jenhagg commented Jun 3, 2021

Purpose

Add the property to grid and mock grid classes as it's commonly used and set at init time.

What the code is doing

Simplify the grid constructor by adding self.grid_model as an instance variable - no need to try/catch to set ModelImmutables. Also set the property in MockGrid since it's already available.

Testing

Existing unit tests

Time estimate

5 min

@jenhagg jenhagg requested review from danielolsen and rouille June 3, 2021 01:08
@jenhagg jenhagg self-assigned this Jun 3, 2021
@@ -176,7 +176,7 @@ def get_branches_by_area(grid, area_names, method="either"):

branch = grid.branch
selected_branches = set()
grid_model = grid.get_grid_model()
grid_model = grid.grid_model
for a in area_names:
load_zone_names = area_to_loadzone(grid_model, a)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use grid.grid_model here and remove grid_model = grid.grid_model since there is the only place where it is used

Copy link
Collaborator

@rouille rouille 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

ng_plant_id = _select_plant_id("ng")
coal_plant_id = _select_plant_id("coal")
dfo_plant_id = _select_plant_id("dfo")
hydro_plant_id = _select_plant_id("hydro")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

@jenhagg jenhagg merged commit 4413c12 into develop Jun 3, 2021
@jenhagg jenhagg deleted the jon/mock branch June 3, 2021 20:52
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.

2 participants