Skip to content

Suggesting improvements for bar plot orientation behaviour. #7994

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

Closed
ghost opened this issue Jan 30, 2017 · 5 comments
Closed

Suggesting improvements for bar plot orientation behaviour. #7994

ghost opened this issue Jan 30, 2017 · 5 comments
Labels
API: consistency status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. status: inactive Marked by the “Stale” Github Action

Comments

@ghost
Copy link

ghost commented Jan 30, 2017

Bug report

Bug summary

Guided by this tutorial example user could expect that the same applies for any graphing function with the keyword orientation. This isn't the default case when dealing with bar. Specifying orientation keyword does not produce a rotated bar graph without manually setting the required parameters to avoid an error. To achieve the desired result barh has to be used.

Code for reproduction

import matplotlib.pyplot as plt

#import numpy as np

#N = 5
#img = np.random.randn(N, N)
#xs = img.sum(axis=0)
#ys = img.sum(axis=1)
#bins = np.arange(0, N, 1)

img = [[-0.13448386, -0.92189708, -0.3883107 ,  1.83910067,  0.46092219],
       [-0.29775299,  0.4623531 , -0.53428658,  0.82103232, -0.66423673],
       [ 1.4559248 , -0.74152634, -0.10229999,  1.12182625, -0.20793333],
       [-1.38758209,  0.61128182, -1.34506956, -0.5511364 ,  0.50791828],
       [ 1.29593156,  0.28209814,  1.60556993,  0.71651822,  0.11639   ]]

xs = [ 0.93203742, -0.30769036, -0.7643969 ,  3.94734105,  0.21306041]
ys = [ 0.85533123, -0.21289089,  1.5259914 , -2.16458795,  4.01650784]
bins = [0, 1, 2, 3, 4]

fig = plt.figure()
plt.subplot(221)
plt.bar(left=None, height=0.8, bottom=bins, width=ys,
          orientation="horizontal")

plt.subplot(222)
plt.bar(bins, ys, orientation="horizontal")

plt.subplot(223)
plt.barh(bins, ys)

plt.subplot(224)
plt.bar(bins, xs)

plt.show()

Actual outcome

Current output for the above code will fail with an error.

Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/matplotlib/axes/test.py", line 18, in <module>
    plt.bar(bins, ys, orientation="horizontal")
  File "/usr/local/lib/python2.7/dist-packages/matplotlib/pyplot.py", line 2705, in bar
    **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/matplotlib/__init__.py", line 1892, in inner
    return func(ax, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/matplotlib/axes/_axes.py", line 2075, in bar
    "be length %d or scalar" % nbars)
ValueError: incompatible sizes: argument 'left' must be length 1 or scalar

Expected outcome

  • First graph (subplot 221, top left) will not produce an error and represents the currently correct usage with orientation horizontal. It's here to show that this isn't a bug, just weird usage (API).
  • Second graph (subplot 222, top right) will produce said error. The error happens because the default parameter order is bar(self, left, height, width=0.8, bottom=None, **kwargs): so left and height are set by default instead of being dependent on orientation keyword in which case sometimes bottom and width parameters should be set instead, and height should become width. Error is raised when the method bar checks data for consistency but doesn't find any.
  • Third graph (subplot 223, bottom left) output should be the same as first two - nothing should be broken for backward compatibility issues as well as the fact that histogram with bar option and horizontal orientation secretly calls barh for drawing.
  • Fourth graph (subplot 224, bottom right) is the vertical orientation and should display correctly after the changes.
    Four graphs: three identical horizontal graphs and one vertical graph.

Proposed change

In the case when bar is called with left and horizontal orientation:

  • values in bottom and left parameters should be exchanged
  • values in width and height should be exchanged

Changes affect the axes/_axes.py file line no. 2034 where I propose that the values are manually exchanged, as in the code below, between the parameters before _process_unit_info is called, that is immediately after elif orientation == "horizontal" line:

if _left is not None:
    _bottom, bottom, _left, left = _left, left, _bottom, bottom
    width, height = height, width

Once this is implemented the listed examples above produce the expected result.

As a final thought I believe that solution is a bit "hackish" because it makes it harder to track data in the code for future devs/users as the switch happens relatively late inside bar. On the other hand its more user friendly because it reduces the amount of knowledge users have to have about how mpl works on the inside and makes it possible to more intuitively work with the mpl's API. In practice I believe code should be refactored to remove barh and standardize the orientation keyword to work even in the cases of line plots or scatter plots. In principle this would consist of just a function that would switch the x and y data parameters. This would not be a very friendly change, and a bit of an overkill, as old code depending on barh would not work anyomre and at the same time a lot of mpl's code already seems to depend on the bar barh difference to work properly.

Additionally, I have no idea if this change breaks something else, but so far it dosen't seem to have hurt my installation very much.

Matplotlib version

  • Matplotlib version 2.0.0, installed from pip, numpy version 1.7.1
  • Python 2.7.6

Sorry if this was an issue before, I looked but found none.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jan 31, 2017
@tacaswell
Copy link
Member

If we were starting from scratch, that is probably a good idea, however, this change would break anyone who is currently working around this issue and using the 'orientation' directly.

I would almost be inclined to go the other way and remove the orientation kwarg and have bar and barh. Just as inputs changing the meaning / types of output is an anti-pattern, I think having kwargs adjust the meaning of inputs is also an anti-pattern.

@ghost
Copy link
Author

ghost commented Jan 31, 2017

@tacaswell
I'm not sure if droping the kwarg is an option as barh depends on it. barh is literally just a call to bar with the parameters filled out as in my subplot 221. Additionally, hist of type "bar" and horizontal orientation just changes _barfunc = self.bar for _barfunc = self.barh therefore is indirectly dependant on what happens there.

Going the other way, in the sense of making the parameter orientation hidden/private, is definitely the more practical approach as I can't really see a lot of people using the orientation kwarg directly. Setting the left to anything else except None or an list of length nbars would fail and either option is fairly unintuitive and the only to find out about it is to read the source-code. Majority of SO answers and search engine results are barh related. Those that are using it would not have to seriously change their code either as the change requires only a pre-pending of _ or __.

Another option would be setting all default parameters to None (in which case current checks would catch data inconsistencies) and continue with the proposed change anyway. Off the top of my head I can't see it breaking any existing code. Both cases of calling the method, positional arguments or explicitly named, would continue functioning as expected (as long as their order is not changed). It would limit experience with IDEs with autocomplete pop-up menus.

The anti-pattern issue is the unfortunate naming of left and height instead of (x, y), (position, bin_value) or (bin_loc, bin_cout) value pairs which would imply independence of coordinate axes orientation choice.

@anntzer
Copy link
Contributor

anntzer commented Jan 31, 2017

To make orientation private you can just have both bar and barh call an inner private function (the current bar).

See also #7966 for my proposal regarding the handling of changes in argument names.

xref #6569 for axes swapping.

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

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 github-actions bot added the status: inactive Marked by the “Stale” Github Action label Apr 6, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2023
@anntzer
Copy link
Contributor

anntzer commented May 9, 2023

Superseded by #17322/#17504.

@rcomer rcomer added the status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: consistency status: closed as inactive Issues closed by the "Stale" Github Action. Please comment on any you think should still be open. status: inactive Marked by the “Stale” Github Action
Projects
None yet
Development

No branches or pull requests

3 participants