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

Improve flexibility of map plots #199

Merged
merged 7 commits into from
Jan 6, 2021
Merged

Conversation

danielolsen
Copy link
Contributor

Purpose

  • Add more flexibility to generate plots with user-defined color ranges. This is useful when comparing two scenarios against each other and trying to highlight the meaningful differences.
  • Other improvements to organization/clarity of related code.

What the code is doing

  • Adding lmp_min and lmp_max parameters to postreise.plot.plot_lmp_map.map_lmp, with default values equal to the previous hardcoded values.
  • Adding vmin and vmax to postreise.plot.plot_utilization_map.map_risk_bind and postreise.plot.plot_utilization_map.map_utilization, with defaults equal to data min/max (same as before)
    • Some simplification of code in map_utilization.
  • Moving postreise.plot.plot_carbon_map.get_borders to postreise.plot.projection_helpers.project_borders
    • Also refactoring this function for simplicity/clarity and updating references to this function in other modules.
  • Docstrings updates

Testing

Relevant notebooks in postreise/plot/demo run successfully.

Time estimate

15-30 minutes. New features are pretty minimal, the rest of the changes are mostly reorganization.

@danielolsen danielolsen self-assigned this Jan 6, 2021
Copy link
Collaborator

@jenhagg jenhagg 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, much cleaner

@victoriahunt
Copy link
Contributor

Thanks for making these changes! Ran the demo note book changing lmp max and min and it's working well. Top is with defaults (min = 20, max = 40), bottom image is with min = 25, max 35.
bokeh_plot (52)

bokeh_plot (51)

@danielolsen danielolsen force-pushed the daniel/plot_flexibility branch from eb26241 to 0a056ec Compare January 6, 2021 21:18
Copy link
Contributor

@victoriahunt victoriahunt left a comment

Choose a reason for hiding this comment

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

Thank you for making these changes to make the code more flexible and cleaner. All looks good.

:return: (bokeh.layout.row) bokeh map visual in row layout
:param inf/float lmp_min: minimum LMP to clamp plot range to.
:param inf/float lmp_max: maximum LMP to clamp plot range to.
:return: (*bokeh.models.layout.Row*) bokeh map visual in row layout
"""
if us_states_dat is None:
us_states_dat = us_states.data

bus = project_bus(s_grid.bus)
lmp_split_points = list(range(0, 256, 1))
Copy link
Collaborator

@rouille rouille Jan 6, 2021

Choose a reason for hiding this comment

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

Are we ever going to modify the above range? if not it could be define in def _get_bus_legend_bars_and_labels and we don't have to propagate it from function to function. Or we could make it a global variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Previously, it had been passed to both _construct_bus_data and _construct_shadowprice_visuals (although not used in _construct_bus_data as of the code in develop right now). I will move it as you suggest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Saves us 8 lines too!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I felt the linter going faster in the checks

@danielolsen danielolsen merged commit e9af2a6 into develop Jan 6, 2021
@danielolsen danielolsen deleted the daniel/plot_flexibility branch January 6, 2021 23:44
@rouille rouille restored the daniel/plot_flexibility branch January 12, 2021 22:32
@rouille rouille deleted the daniel/plot_flexibility branch January 12, 2021 23:29
@ahurli ahurli mentioned this pull request Mar 16, 2021
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.

4 participants