Integrate spot check code for plotting multiple scenarios

[merrielle]

This pr integrates our spot check code into PostREISE. I decided I wanted to avoid messing with analyze_pg so as to not step on any toes. We can refactor everything together in the future. I intended to use this code structure as a base for future plotting code. Each graph type has its own file with only one (or two, for bar and hbar graphs) public facing functions. There is a common file for helper functions where the plotting code overlaps.

For the record I am not a fan of posting huge reviews like this -- I think it's better for everyone's sanity to break tasks into small bits and keep PRs around 200 lines if possible. So I appreciate your work in reviewing this. :)

[kasparm]

Thanks for the work. Could you please read through our guidelines(https://github.com/intvenlab/REISE/wiki/Software-Development-Guidelines) and follow the conventions please. Especially how to document the code and the format of the commit messages.

[kasparm]

@merrielle you can rebase to develop now.

[rouille]

I did the following:

from postreise.plot.multi.plot_bar import plot_bar

plot_bar('Western', ['87'])

and I got a plot for all the load zones in the Western interconnection. I noticed that the legend/title are overlapping. This is shown below for Idaho.

I understand that the routines are built to compare multiple scenarios. Should we:

• raise an error if the list of scenario ids has only one element.
• or we want to have a bar plot for a single scenario. In that case, we need to rearrange the titles and print only one scenario name at the bottom.

[rouille]

I noticed some style issues (PEP8 and others):

• Every line (code, comment, docstring) should be < 80 characters.
• Unused import statement:
  numpy in plot_bar
pandas in plot_helpers
ALL_RESOURCE_TYPES in plot_pie
matplotlib.pyplot and numpy in plot_shortfall
• Issue with docstring in:
  plot_hbar (no parameter interconnect)
  unit_conversion (return statement needs to be located after the docstring)
  _construct_pie_visuals (no parameter ax_data)
• Also, I think we want to populate __all__ in the init.py

[rouille]

Do you think it would be possible to have a keyword in the routines to set the scenario nane? When not None it will not fetch the horibble scenario name. To illustrate, scenario #87 has name: WesternBase_2016_noHVDC_Final_2019Sep. I would say it is better to have something different in the titles or legends.

[rouille]

It is possible to pass a custom_data dictionary in plot_bar and plot_pie. I believe this is used to produce the kind of plot that compares our generation levels wiith NREL's. I was wondering if we should not have a data folder in postreise/plot/multi where the NREL (and other exterior data are stored) and write a helper function that would read the data and produce a custom_data dictionary. I believe this would be useful to anybody who want to make this plot.

[rouille]

Regarding the tests, is there any reason why you don't use the unittest unit testing framework?

[rouille]

As mentioned during the meeting this morning, I think it would be nice that the user can select the start and end dates for the chart. In the past we have been looking at generation level for the summer, winter or even a single month.

[merrielle]

Re: plotting a single scenario
I think that's functionality we should definitely add, but maybe in a future PR. For now I'll add an error if there's only one scenario.

Re: style issues
Will fix! Also, is there a reason we add __all__ to the init file? I know you don't have to but maybe there's something I'm missing.

Re: scenario name
Yeah... I was thinking about doing that and decided to leave it alone for now. I can add it since it seems important

Re: NREL custom data
That's a great point and I think it would be useful. I'd want to think more about where that data is stored. After this PR is done I'll make a new PR to add it.

Re: tests
Software Dan and I think pytest is better because you need to write less boilerplate code in order to write tests. I think by this point we've told everyone via the slack and in person. Luckily we don't need to change any of the old unittest tests because pytest can run those as-is.

[kasparm]

@dmuldrew please update the testing guideline to reflect the fact that we use pytest.

[merrielle]

@rouille I've finished all the fixes you requested :)

[rouille]

Very nice! Thanks a lot, it is fantastic.

[kasparm]

Do we have any demo or readme for this?
What is the plan? This is code that researcher going to us.

[kasparm]

Great work.
There are a few things we need to synchronize when we refactor. The part of the constants are covered in the gird object.
Also please add the readme to your todo list. For now we can merge this.