NREL / floris

A controls-oriented engineering wake model.
http://nrel.github.io/floris
BSD 3-Clause "New" or "Revised" License
215 stars 156 forks source link

Bug report: `WindRose.plot()` argument `legend_kwargs` are said to be passed to `legend` in the doc strings, but they are passed to `colorbar` instead #1023

Open jaredthomas68 opened 2 weeks ago

jaredthomas68 commented 2 weeks ago

Bug report: WindRose.plot() argument legend_kwargs are said to be passed to legend in the doc strings, but they are passed to colorbar instead

WindRose.plot() argument legend_kwargs are said to be passed to legend in the doc strings, but they are passed to colorbar instead. This makes usage non-intuitive.

How to reproduce

Pass typical legend arguments to WindRose.plot()


    import matplotlib.pyplot as plt

    wind_data = fmodel.wind_data

    time_series = TimeSeries(
        wind_directions=wind_data.wind_directions,
        wind_speeds=wind_data.wind_speeds,
        turbulence_intensities=wind_data.turbulence_intensities,
    )

    wind_rose = time_series.to_WindRose(wd_edges=np.arange(0, 360, 3.0), ws_edges=np.arange(2, 20, 2.0))

    fig, ax = plt.subplots(1, figsize=(4,4), subplot_kw={'projection': 'polar'})

    ax = wind_rose.plot(ax=ax, wd_step=10, legend_kwargs={"title": "Wind Speed (m/s)", "frameon": True})

Relevant output

  File "floris/floris/wind_data.py", line 782, in plot
    ax.figure.colorbar(sm_ws, ax=ax, **legend_kwargs)
  File "matplotlib/figure.py", line 1253, in colorbar
    cb = cbar.Colorbar(cax, mappable, **{
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Floris version

4.2

System Information

misi9170 commented 1 week ago

Hi @jaredthomas68 , thanks for raising this. I believe the problem was introduced in #969. I think you can get around this for now by setting legend_kwargs to {"label": Wind Speed (m/s)"} (and dropping the frameon argument). But this is a user-interface change that we should have handled more thoroughly in a minor release, so I'll look into a more robust bugfix.

misi9170 commented 1 week ago

Hi @jaredthomas68 , I've believe I've now addressed this in #1028