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.

Sign up for GitHub

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

Jump to bottom

Refactor states plotting #306

Merged
merged 13 commits into from
Jun 17, 2021
Merged

Refactor states plotting #306

merged 13 commits into from
Jun 17, 2021

Conversation

rouille
Copy link
Collaborator

@rouille rouille commented Jun 10, 2021

Pull Request doc

Purpose

Break the function plotting states on a figure into several functions. Each function has a single responsibility:

  • Plot borders
  • Color states
  • Create tooltips
  • Add legend

Closes #305

What the code is doing

Create functions that operate on region (states) on abokeh figure

Testing

Run tests and notebooks

Where to look

The postreise.plot.plot_states module. We just use the new functions in the other files.

Usage Example/Visuals

How the code can be used and/or images of any graphs, tables or other visuals (not always applicable).

Time estimate

15min

@rouille rouille self-assigned this Jun 10, 2021
@rouille rouille changed the title Ben/canvas Refactor states plotting Jun 10, 2021
BainanXia
BainanXia reviewed Jun 10, 2021
View reviewed changes
postreise/plot/canvas.py Outdated Show resolved Hide resolved
BainanXia
BainanXia approved these changes Jun 10, 2021
View reviewed changes
Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

Very nice. This is much cleaner and better organized.

ahurli
ahurli reviewed Jun 10, 2021
View reviewed changes
postreise/plot/plot_states.py Outdated Show resolved Hide resolved
postreise/plot/plot_states.py Outdated Show resolved Hide resolved
postreise/plot/plot_capacity_map.py Show resolved Hide resolved
postreise/plot/plot_capacity_map.py Outdated Show resolved Hide resolved
@danielolsen danielolsen mentioned this pull request Jun 11, 2021
Merged
@rouille rouille force-pushed the ben/canvas branch 2 times, most recently from 923043b to b635c52 Compare June 15, 2021 20:58
@rouille
Copy link
Collaborator Author

rouille commented Jun 15, 2021

I have improved this PR by:

  • creating a postreise.plot.check module that encloses function that ensure that keyword arguments that are passed to a user facing function are valid
  • testing this function
  • using this function in the user facing functions building maps

jenhagg
jenhagg approved these changes Jun 16, 2021
View reviewed changes
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.

LGTM

@rouille rouille merged commit 6f46f6e into develop Jun 17, 2021
@rouille rouille deleted the ben/canvas branch June 17, 2021 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahurli ahurli ahurli left review comments

@jenhagg jenhagg jenhagg approved these changes

@BainanXia BainanXia BainanXia approved these changes

@danielolsen danielolsen Awaiting requested review from danielolsen

Assignees

@rouille rouille

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor states plotting
4 participants
@rouille @BainanXia @jenhagg @ahurli