-
Notifications
You must be signed in to change notification settings - Fork 13
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
Integrate spot check code for plotting multiple scenarios #44
Conversation
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. |
73793a5
to
34107e4
Compare
6467f45
to
9ec237e
Compare
@merrielle you can rebase to develop now. |
6167b77
to
290d9bc
Compare
I noticed some style issues (PEP8 and others):
|
Do you think it would be possible to have a keyword in the routines to set the scenario nane? When not |
It is possible to pass a |
Regarding the tests, is there any reason why you don't use the |
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. |
Re: plotting a single scenario Re: style issues Re: scenario name Re: NREL custom data Re: tests |
@dmuldrew please update the testing guideline to reflect the fact that we use pytest. |
@rouille I've finished all the fixes you requested :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Thanks a lot, it is fantastic.
Do we have any demo or readme for this? |
rebase: Add all analyze code in one file, refactor code for better documentation and testability rebase: Break everything into seperate files by graph type. Add mocks and tests
fix: Small tweaks before PR refactor: Change test dir to tests. Make small changes to scenario import and tweaks to visuals fix: Deepcopy mock objects to prevent test contamination fix: Remove mock objects from global namespace style: Follow pycodestyle and pep8 guidelines
refactor: split shortfall inputs and plot helpers fix: keep bar plots from overlapping, fix target line position for shortfall
1ac4ca2
to
2b9b6a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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. :)