wrobstory / vincent

A Python to Vega translator
MIT License
2.04k stars 227 forks source link

Add Data Labels to GroupedBar Chart #122

Closed kennethdamica closed 9 years ago

kennethdamica commented 10 years ago

This adds data labels to the GroupedBar chart. Added additional parameters data_labels (boolean to turn on/off), color, fontsize, and baseline (vertical alignment).

Examples:

group = vincent.GroupedBar(df_2, data_labels= True, label_color='#000000', fontsize= 11)
group2 = vincent.GroupedBar(df_2[:3], data_labels= True, label_color='#000099', fontsize= 18, baseline='bottom')
wrobstory commented 10 years ago

Gonna try to get this merged in tonight, stay tuned.

wrobstory commented 9 years ago

So: this is great, just one comment: can you move the keyword arguments into the GroupedBar constructor itself? I'd like to keep the Chart constructor as clean as possible.

Also, I wouldn't blame you if you say "This has been sitting here for 4 months, f that", in which case I'll merge it and move them around tomorrow :) Just wanted to give you the chance.

Apologies for the huge delay- I'm going to try and pick up speed on this project again with the new Vega and IPython 3.0 releases on the horizon.

kennethdamica commented 9 years ago

I moved the kwargs into GroupedBar. However, this led me to notice that 'data' is a keyword arg that is used as a positional arg in all the examples. I left 'data' out of GroupedBar, which led to 'TypeError: got multiple values for argument'. Do you think I should add the 'data' arg to GroupedBar or are the examples wrong?

wrobstory commented 9 years ago

Oooooo thats kind of a gross thing I did there. Data should have been a positional arg from the beginning, but the API has been in the wild for a while with it as a keyword argument, and changing it might cause some unfortunate backwards breakages.

I think the easiest thing here is to add data as a keyword to GroupedBar and pass it to the chart constructor.

 def __init__(self, data=None, data_labels=False, label_color='#000000', 
                     fontsize=12, baseline='top',
                     *args, **kwargs):

 super(GroupedBar, self).__init__(data=data, *args, **kwargs)
kennethdamica commented 9 years ago

Okay great. It seems to work! Sorry about all the commits. I am realizing that I am a git noob.

wrobstory commented 9 years ago

Merged! Thank you for your contribution and patience!