-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add functionality to plot bar and barh with string labels (Implement #2516) #4266
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
Conversation
The Travis docs build failure is not an issue with this Pull request. Fixed by #4267 |
@@ -1960,6 +1965,11 @@ def make_iterable(x): | |||
left *= nbars | |||
if len(height) == 1: | |||
height *= nbars | |||
if nbars and cbook.is_sequence_of_strings(bottom): | |||
ticks_loc = [i + height[i]/2 for i in range(nbars)] |
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.
I would do this as [i + h / 2 for i, h in enumerate(height)]
, same for the logic in bar
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.
Thanks, this looks a lot better now! Fixed via 1627e8e
👍 for the idea, I am a bit wary of overloading |
You're right. To address this I added a check and raised ValueError if bottom (bar) or left (barh) are a list of strings. Perhaps we should more explicitly state this in the docstring as well. If you have any suggestions on how to improve this let me know! |
Sorry, I transposed the issue. Now you can't specify both the labels and non-unit bar spacing which is much worse. |
Ah, I see what you mean. Perhaps a better way to do this is to not touch left and bottom and instead add an extra kwarg 'labels' or something similar? The use case I am thinking about is: bar([1,2,3], [1,1,1], labels=['a', 'b', 'c']) |
Yes, I think that is a better route. I would also not move the ticks relative to the bar unless explicitly asked to. Both |
@umairidris You should also have a look at how boxplot handles this sort of thing. iirc |
Thanks for the feedback and advice @tacaswell! I have addressed it via commit c04bf79. A few points I should mention:
I hope I addressed your concerns. Please let me know if you have any further suggestions! |
This looks good, just one code comment. Can you add an entry in https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new so that this gets advertised? |
@@ -1926,6 +1930,8 @@ def make_iterable(x): | |||
width = make_iterable(width) | |||
_bottom = bottom | |||
bottom = make_iterable(bottom) | |||
_tick_label = tick_label |
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.
I would do this as
label_ticks_flag = not tick_label is None
Which I think is a bit clearer.
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.
tick_label is not None, please
On Mar 29, 2015 11:33 PM, "Thomas A Caswell" notifications@github.com
wrote:
In lib/matplotlib/axes/_axes.py
#4266 (comment):@@ -1926,6 +1930,8 @@ def make_iterable(x):
width = make_iterable(width)
_bottom = bottom
bottom = make_iterable(bottom)
_tick_label = tick_label
I would do this as
label_ticks_flag = not tick_label is None
Which I think is a bit clearer.
—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/4266/files#r27365250.
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.
erm, yes, what @WeatherGod said
@umairidris This needs to be rebased, not sure why. |
@tacaswell Done! My branch had the old docstrings (not numpy format) which was causing merge conflicts. All fixed now. :) Umair I. |
Great. I will merge this as soon as travis passes again. |
ENH : pass list of strings to label bar/barh plots Add functionality to plot bar and barh to label bars with list strings (closes #2516)
This patch is an attempt to add the functionality to pass in strings to label the axis of a bar graph (issue #2516). The patch shouldn't break any existing API. I also center the labels based on the width/height passed for a cleaner more professional look (please view the tests). I am happy to hear any feedback and/or concerns!
Cheers!
Umair