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

Add utilization map to tutorial #291

Merged
merged 4 commits into from
May 11, 2021
Merged

Add utilization map to tutorial #291

merged 4 commits into from
May 11, 2021

Conversation

rouille
Copy link
Collaborator

@rouille rouille commented May 11, 2021

Pull Request doc

Purpose

Add utilization map to tutorial. Closes #290.

What the code is doing

N/A

Testing

Code snippet has been run in the plot_tutorial notebook.
You can run tox in the docs repository to see the updates

Where to look

  • fix a bug in the postreise/plot/plot_utilization_map module. Keywords were not correctlypassed to plot_states function
  • update the utilization_map_demo notebook
  • add utilization map to tutorial
  • update tutorial notebook with new plot

Time estimate

5 min

@rouille rouille added the documentation Documentation related to package label May 11, 2021
@rouille rouille requested review from danielolsen and BainanXia May 11, 2021 05:39
@rouille rouille self-assigned this May 11, 2021
docs/plot.rst Outdated
from powersimdata.utility.helpers import PrintManager
from powersimdata.scenario.scenario import Scenario
from powersimdata import Scenario
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two imports, which one should go first or it doesn't matter according to flake8?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is a good question. I don't know, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've no idea. Flake8 is not complaining either way. I guess we are following pep8 only, right? According to style, pep8 enforces groups without enforcing the order within the groups

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I switched it since other code snippets have it first

@BainanXia
Copy link
Collaborator

Wrong description of Purpose due to copy/paste?

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.

Looks good.

@rouille rouille merged commit d15f32f into develop May 11, 2021
@rouille rouille deleted the ben/tutorial branch May 11, 2021 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation related to package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add utilization map to tutorial
2 participants