-
Notifications
You must be signed in to change notification settings - Fork 18
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
Feature: Energy ratio polar plots and wind speed distributions #72
Feature: Energy ratio polar plots and wind speed distributions #72
Conversation
@Bartdoekemeijer nice! What version of matplotlib are you running? I found that I needed to update so that ax.legend() would accept the "ncols" keyword argument, and once updated, I found that ax.grid() no longer accepted the "b" keyword argument (this seems to have been replaced with "visible"). I have gone ahead and changed the "b" to "visible" and committed to your branch. This seems to run fine for me now with the latest matplotlib (3.7.1). (I was on 3.5.3 before, an the "ncols" keyword argument was failing). We don't specify an exact version for matplotlib in the setup/requirements, which to me is fine---users should just grab the latest. |
@Bartdoekemeijer @paulf81 I see that there is now a merge conflict due to fig no longer being passed out from vis.plot()---Bart, are you fine with also removing the plt.gcf() return from vis.plot()? I'll let you manage the conflict, but I'll approve now anyway. |
This is really cool @Bartdoekemeijer thanks for pushing this up! I downloaded the code and ran the two modified examples and those looked nice (following upgrading matplotlib like Misha, maybe we should boost the requirement?) Sorry about the merge conflict! I can help with that if you like. Then also, does the example wake_steering_example.py work for you? I'm getting an error on this one thinking it could be to do with the new bar plots per wind speed? Then final comment, when we plot two dfs, do you think we should still have side-by-side bars as an option to show balancing? |
Thanks for the feedback @misi9170 @paulf81! I have updated the requirements to use at least my version of |
That function still exists. The thing now is that it reduces to single bars if the two underlying distributions are the same. I think keeping it makes sense because it can give a good insight into the underlying distributions. Also, I merged the latest develop branch and things should be good to merge now, if you all agree. |
HI @Bartdoekemeijer when I try running: examples/energy_ratio/wake_steering_example.py I get an error: ValueError: shape mismatch: objects cannot be broadcast to a single shape. Mismatch is between arg 0 with shape (29,) and arg 1 with shape (20,). |
@paulf81 good catch, OK this should do it now! I do wonder how to deal with the bar plots when using |
hi @Bartdoekemeijer it runs for me now, thanks! So looking at this plot: What if we mapped which df each bar belonged to a color, and then split wind speeds on alpha? Then maybe we could have it both ways? |
@paulf81 how about this, using colors to correspond the energy ratios to the bar plots and using hatching and transparency to distinguish between wind speed bins? |
I think it's great! Thank you @Bartdoekemeijer !! @misi9170 good for you if I merge? |
@paulf81 yes, go for it |
Hold on. I still have to push up these actual commits! |
@Bartdoekemeijer haha oops, thanks |
…y ratio visualization
OK good to go now, I think! :) |
Feature or improvement description
This PR adds two useful functionalities to the energy ratio plotting library:
Combining (1) and (2) turns the bin count plot into a proper wind rose plot. Here's an example from the
flasc_cookiecutter_template
repository with the new functionality:Related issue, if one exists
None.
Impacted areas of the software
Energy ratio classes and energy ratio visualization classes. No fundamental changes in how energy ratios are calculated or used.
Additional supporting information
N/A.
Test results, if applicable