-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
adding a diverging bar example to the horizontal bar documentation #4994
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
An example of a butterfly chart/diverging bar chart has been added.
631756e
to
6c2426a
Compare
the only thing we changed is horizontal-bar-charts.md |
Thanks @gvwilson for merging plotly/datasets#63. We believe this example and its sister commit #4984 now follow the applicable Plotly best practices and look forward to getting feedback from @LiamConnors or another appropriate reviewer! |
@LiamConnors can you please review for the Plotly.py 6.1 release? |
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.
Looks good @rl-utility-man. Thanks for creating this PR.
Just left a few small comments and questions
doc/python/horizontal-bar-charts.md
Outdated
fig = go.Figure() | ||
# this color palette conveys meaning: blues for negative, reds for positive, gray for Neither Agree nor Disagree | ||
color_by_category={ | ||
"Strongly Agree":'darkblue', | ||
"Agree":'lightblue', | ||
"Disagree":'orange', | ||
"Strongly Disagree":'red', | ||
"Neither Agree nor Disagree":'gray', | ||
} |
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.
Should this say "blues for positive" and "reds for negative"?
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.
Yes, corrected.
Co-authored-by: Liam Connors <[email protected]>
Thanks @LiamConnors for spotting these issues. I hope I have resolved them. Normally, @SimaRaha who did so much of this would have responded, but she's otherwise committed this week. #4984 is a continuation of this example and I would be delighted if we considered them together. |
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.
Just a few small comments. Thanks for opening this PR
adding python decorator Co-authored-by: Liam Connors <[email protected]>
Co-authored-by: Liam Connors <[email protected]>
Glad @SimaRaha and I could help! Good suggestions @LiamConnors. I accepted and committed them! Looking forward to merging this one and considering the continuation of this example in #4984 |
doc/python/horizontal-bar-charts.md
Outdated
Diverging bar charts show counts of positive outcomes or sentiments to the right of zero and counts of negative outcomes to the left of zero, allowing the reader to easily spot areas of excellence and concern. This example leaves the number of people offering a neutral response implicit because the categories add to 100%. | ||
|
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.
This looks good and it's ready to merge, but I just want to check the wording on "This example leaves the number of people offering a neutral response implicit because the categories add to 100%." Should this say "
This example leaves the number of people offering a neutral response implicit because the categories don't add up to 100%."
Is that how we know there are neutral responses that are not shown here?
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.
Yes! Good point. I tried a rewrite of your wording that's hopefully a little more direct. I'm open to your wording as well.
@LiamConnors I'm thrilled to see this merged. I just realized we requested that it be merged into "main" rather than "doc-prod" so it's not currently showing up on the website. Please advise about whether and how I should address that. Should I e.g. open a PR to merge this or main into doc-prod? |
@rl-utility-man, if you want to open a PR that has these commits and targets doc-prod, I'll merge it in. Let me know if I can help. Thanks again for this PR |
@LiamConnors I just created #5148 to insert the same change into doc-prod. I just copied the change from the "files changed" tab. Much appreciated. I enjoyed working with you and @SimaRaha on this PR and very much appreciate your help. We've also responded to your comments on its sibling PR, #4984 |
Applying changes that #4994 made to "main" to "doc-prod"
Documentation PR
doc/README.md
filedoc-prod
branch OR it targets themaster
branchpx
example if at all possibleplotly.graph_objects as go
/plotly.express as px
/plotly.io as pio
df
fig = <something>
call is high up in each new/modified example (eitherpx.<something>
ormake_subplots
orgo.Figure
)fig.add_*
andfig.update_*
rather thango.Figure(data=..., layout=...)
in every new/modified examplefig.add_shape
andfig.update_xaxes
are used instead of bigfig.update_layout
calls in every new/modified examplefig.show()
is at the end of each new/modified exampleplotly.plot()
andplotly.iplot()
are not used in any new/modified exampleCode PR
plotly.graph_objects
, my modifications concern thecodegen
files and not generated files.modified existing tests.
new tutorial notebook (please see the doc checklist as well).
-->