Add waterfall chart example #2621
Conversation
There was a problem hiding this comment.
Thanks for creating this PR @yanghung ! I made a few minor comments on coding style , but other than that this looks good to me! Could you address them and ping me after so that I can review again?
|
|
||
|
|
|
|
||
|
|
||
| ## Compared to the original example, we have to add some more fields to help with color labeling and ordering the bars | ||
| the_data=[ |
There was a problem hiding this comment.
| the_data=[ | |
| the_data = [ |
Could you add spaces around all variable assignment = like this?
| calc_lead="datum.window_lead_label === null ? datum.label : datum.window_lead_label" | ||
| , calc_prev_sum="datum.label === 'End' ? 0 : datum.window_sum_amount - datum.amount" |
There was a problem hiding this comment.
| calc_lead="datum.window_lead_label === null ? datum.label : datum.window_lead_label" | |
| , calc_prev_sum="datum.label === 'End' ? 0 : datum.window_sum_amount - datum.amount" | |
| calc_lead="datum.window_lead_label === null ? datum.label : datum.window_lead_label", | |
| calc_prev_sum="datum.label === 'End' ? 0 : datum.window_sum_amount - datum.amount | |
| " |
Could you make this change everywhere (having comma last instead of first) so that the style is consistent with the rest of the Altair documentation?
| ) | ||
|
|
||
| ## layer everything together | ||
| (bar+rule+text_pos_values_top_of_bar+text_neg_values_bot_of_bar+text_bar_values_mid_of_bar).properties(width=800, height=450) |
There was a problem hiding this comment.
Since this line is so long I would write it like this instead:
alt.layer(
bar,
rule,
text_pos_values_top_of_bar,
text_neg_values_bot_of_bar,
text_bar_values_mid_of_bar
).properties(
width=800,
height=450
)| ) | ||
|
|
||
| ## the "rule" chart is for the lines that connect bars each month | ||
| rule=base_chart.mark_rule(color='#404040', opacity=0.9, strokeWidth=2, xOffset=-25, x2Offset=25, strokeDash=[3,3]).encode( |
There was a problem hiding this comment.
| rule=base_chart.mark_rule(color='#404040', opacity=0.9, strokeWidth=2, xOffset=-25, x2Offset=25, strokeDash=[3,3]).encode( | |
| rule = base_chart.mark_rule( | |
| color='#404040', | |
| opacity=0.9, | |
| strokeWidth=2, | |
| xOffset=-25, | |
| x2Offset=25, | |
| strokeDash=[3,3] | |
| ).encode( |
I would break this up since the line is really long
|
@yanghung When you are addressing the comments I made, could you also rebase this on the latest main branch so that the CI tests above pass (I recently committed a fix)? |
|
If you merge the PR in its current state I can create a follow-up one in the next few days to address the open points. If it doesn't work due to the failed checks I can also copy over the code from here and add @yanghung as an author. |
|
Thanks @binste! Which category is this supposed to be? It's currently in |
|
As it contains quite a few transformations I'd suggest to place it into Advanced Calculations |
No description provided.