matplotlib icon indicating copy to clipboard operation
matplotlib copied to clipboard

hist reverses legend order if histtype is not bar

Open efiring opened this issue 8 years ago • 8 comments

Matplotlib v2.0.x and master on Python 3.5:

x1 = np.random.randn(100)
x2 = np.random.rand(100)
plt.hist([x1, x2], label=['x1', 'x2'], histtype='step')
plt.legend()

Actual outcome hist_reversed_order

Expected outcome

With histtype='bar' the order of the variables is preserved, as I would expect, instead of reversed, as with step.

efiring avatar Apr 10 '17 22:04 efiring

The culprit is the comment at the top of

            # add patches in reverse order so that when stacking,
            # items lower in the stack are plottted on top of
            # items higher in the stack
            for x, y, c in reversed(list(zip(xvals, yvals, color))):
                patches.append(self.fill(
                    x[:split], y[:split],
                    closed=True if fill else None,
                    facecolor=c,
                    edgecolor=None if fill else c,
                    fill=fill if fill else None))
            for patch_list in patches:
                for patch in patch_list:
                    if orientation == 'vertical':
                        patch.sticky_edges.y.append(minimum)
                    elif orientation == 'horizontal':
                        patch.sticky_edges.x.append(minimum)

            # we return patches, so put it back in the expected order
            patches.reverse()

anntzer avatar Apr 12 '17 09:04 anntzer

I think any reasonable fix to this will lead to inversing the order in which the bars are being drawn (which is fine with me -- you'd expect the later datasets to be plotted on top of the earlier ones per standard matplotlib behavior -- but would definitely cause a backcompatibility break).

anntzer avatar Aug 26 '17 02:08 anntzer

This bug is still present in matplotlib 3.7.1 and python 3.11.2. For the reason given by @anntzer above, it becomes even more confusing if one calls plt.legend() separately:

x1 = np.random.randn(100)
x2 = np.random.rand(100)
plt.hist([x1, x2], histtype='step')
plt.legend(['x1', 'x2'])

yields reversed colours and labels: wrong

It's easy to see how this bug could fly under the radar: the swapping of colours and labels in code for a work project this morning confused me for quite some time.

davidjamesweir avatar Mar 27 '23 10:03 davidjamesweir

I believe this is partially addressed by #24759, adjusting @efiring 's example to use plt.legend(reverse=True). Not ideal, but it is something.

The example from @davidjamesweir is a less than great feature of the API where you can pass ax.legend just the labels you want and then we go and look for labelable things and assign the labels in that order. If the plotting calls only produce one Artist this is clear, but if our calls (or a third-party library calls) produce more than one Artist you are now exposed to a slightly fuzzy aspect of our API (is the order we add Artists to the internal lists stable or not?!).

tacaswell avatar Mar 27 '23 14:03 tacaswell

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

github-actions[bot] avatar Mar 27 '24 01:03 github-actions[bot]

The reversal dates back to 2e1a96d1, but I see no obvious reason why that has been done.

One could retain the draw order while reversing the lists by adding minimal zorder (e.g. 1e-12). But that's a bit hacky, and also the draw order being different between types bar and stepfilled is quite surprising. I would consider this buggy behavior even though it seems to have been introduced intentionally. In that sense I'd be fine with just reversing the order and calling it a bug fix.

timhoffm avatar Mar 27 '24 08:03 timhoffm

Time to fix this bug. I guess that most existing code is no longer in use. Suggestion: Let the parameter reverse have None as default = reversing for stacked histograms only, so that most existing code could be fixed by just removing the reverse parameter. Emit a warning for a year or so.

Rainald62 avatar Oct 01 '24 13:10 Rainald62

That migration pattern does not really help much: We've introduced legend(reverse=...) as a general-purpose parameter. It can be used to fix the order of hist, but also generally. To make a smoother hist-migration, we'd have to try and detect whether legend(reverse=True) was used to fix the hist order, i.e. some internal book-keeping that the legend entries all come from a hist call. While it's technically possible IMHO it's not worth the effort.

The slightly better approach would be to introduce a reverse_legend_order parameter (name t.b.d.) on hist, so that we contain the topic of artist ordering in hist().

timhoffm avatar Oct 07 '24 12:10 timhoffm