Add functionality to plot bar and barh with string labels (Implement #2516) by umairidris · Pull Request #4266 · matplotlib/matplotlib

umairidris · 2015-03-23T08:25:01Z

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

jenshnielsen · 2015-03-23T10:26:02Z

The Travis docs build failure is not an issue with this Pull request. Fixed by #4267

tacaswell · 2015-03-23T11:30:42Z

I would do this as [i + h / 2 for i, h in enumerate(height)], same for the logic in bar

Thanks, this looks a lot better now! Fixed via 1627e8e

tacaswell · 2015-03-23T11:33:24Z

👍 for the idea, I am a bit wary of overloading bottom and left in this way as now you can either label them or have non-zero baseline, but not both.

umairidris · 2015-03-23T17:16:00Z

👍 for the idea, I am a bit wary of overloading bottom and left in this way as now you can either label them or have non-zero baseline, but not both.

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!

tacaswell · 2015-03-23T17:22:33Z

Sorry, I transposed the issue. Now you can't specify both the labels and non-unit bar spacing which is much worse.

umairidris · 2015-03-23T17:45:56Z

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'])

tacaswell · 2015-03-23T17:54:58Z

Yes, I think that is a better route. I would also not move the ticks relative to the bar unless explicitly asked to. Both bar* functions have an align kwarg which should still be respected.

tacaswell · 2015-03-25T03:19:09Z

@umairidris You should also have a look at how boxplot handles this sort of thing. iirc bxp has a bunch of tick-management logic at the bottom of it.

umairidris · 2015-03-25T07:53:00Z

Thanks for the feedback and advice @tacaswell! I have addressed it via commit c04bf79.

A few points I should mention:

I wanted to match the 'label' bxpstats key used by bxp. However, label is a kwarg that is already being used by bar. I am not sure what the best name for my added kwarg should be... tick_label (currently used)? bar_label? text_label?
I added the ability to pass in the labels as a single string to remain consistent with the rest of the API. You can now do bar(0, 1, tick_label='a') (please see added test).
I moved the code to set the labels to the bottom of the bar function. This mimics bxp and also fixes interfering with autoscale (self.autoscale_view() needs to be called before the tick labels are set).
As requested, bar* align kwarg is now respected (see updated tests).

I hope I addressed your concerns. Please let me know if you have any further suggestions!

tacaswell · 2015-03-30T03:32:07Z

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?

tacaswell · 2015-03-30T03:33:16Z

I would do this as

label_ticks_flag = not tick_label is None

Which I think is a bit clearer.

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.

erm, yes, what @WeatherGod said

tacaswell · 2015-04-12T00:01:13Z

@umairidris This needs to be rebased, not sure why.

umairidris · 2015-04-12T02:19:04Z

@tacaswell Done! My branch had the old docstrings (not numpy format) which was causing merge conflicts. All fixed now. :)

Umair I.

tacaswell · 2015-04-12T02:39:36Z

Great. I will merge this as soon as travis passes again.

… new entry.

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)

tacaswell added the status: needs review label Mar 23, 2015

tacaswell reviewed Mar 23, 2015
View reviewed changes

tacaswell added this to the next point release milestone Mar 23, 2015

tacaswell added status: needs revision and removed status: needs review labels Mar 24, 2015

tacaswell added status: needs review and removed status: needs revision labels Mar 30, 2015

tacaswell reviewed Mar 30, 2015
View reviewed changes

Add functionality to plot bar and barh with string labels. Add what's…

c2a6da5

… new entry.

tacaswell added a commit that referenced this pull request Apr 12, 2015

Merge pull request #4266 from umairidris/bar_labels

a6af675

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)

tacaswell merged commit a6af675 into matplotlib:master Apr 12, 2015

tacaswell removed the status: needs review label Apr 12, 2015

umairidris deleted the bar_labels branch April 12, 2015 03:35

Uh oh!

Conversation

umairidris commented Mar 23, 2015

Uh oh!

jenshnielsen commented Mar 23, 2015

Uh oh!

tacaswell Mar 23, 2015

Choose a reason for hiding this comment

Uh oh!

umairidris Mar 23, 2015

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Mar 23, 2015

Uh oh!

umairidris commented Mar 23, 2015

Uh oh!

tacaswell commented Mar 23, 2015

Uh oh!

umairidris commented Mar 23, 2015

Uh oh!

tacaswell commented Mar 23, 2015

Uh oh!

tacaswell commented Mar 25, 2015

Uh oh!

umairidris commented Mar 25, 2015

Uh oh!

tacaswell commented Mar 30, 2015

Uh oh!

tacaswell Mar 30, 2015

Choose a reason for hiding this comment

Uh oh!

WeatherGod Mar 30, 2015

Choose a reason for hiding this comment

Uh oh!

tacaswell Mar 30, 2015

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Apr 12, 2015

Uh oh!

umairidris commented Apr 12, 2015

Uh oh!

tacaswell commented Apr 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants