Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Example 2 for Butterfly chart (version2)#4984

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
SimaRaha wants to merge13 commits intoplotly:doc-prod
base:doc-prod
Choose a base branch
Loading
fromSimaRaha:patch-3

Conversation

SimaRaha
Copy link
Contributor

Please uncomment this block and take a look at this checklist if your PR is making substantial changes todocumentation/impacts files in thedoc directory. Check all that apply to your PR, and leave the rest unchecked to discuss with your reviewer! Not all boxes must be checked for every PR :)

If your PR modifies code of theplotly package, we have a different checklist
below :-).

Documentation PR

  • I'veseen thedoc/README.md file
  • [ Yes] This change runs in the current version of Plotly on PyPI and targets thedoc-prod branch OR it targets themaster branch
  • [ N/A instead used plotly.graph_objects ] If this PR modifies the first example in a page or adds a new one, it is apx example if at all possible
  • Every new/modified example has a descriptive title and motivating sentence or paragraph
  • [ Yes] Every new/modified example is independently runnable
  • [ Yes] Every new/modified example is optimized for short line countand focuses on the Plotly/visualization-related aspects of the example rather than the computation required to produce the data being visualized
  • Meaningful/relatable datasets are used for all new examples instead of randomly-generated data where possible
  • [Yes ] The random seed is set if using randomly-generated data in new/modified examples
  • [ Yes] New/modified remote datasets are loaded fromhttps://plotly.github.io/datasets and added tohttps://github.com/plotly/datasets
  • Large computations are avoided in the new/modified examples in favour of loading remote datasets that represent the output of such computations
  • [Yes ] Imports areplotly.graph_objects as go /plotly.express as px /plotly.io as pio
  • Data frames are always calleddf
  • [ Yes]fig = <something> call is high up in each new/modified example (eitherpx.<something> ormake_subplots orgo.Figure)
  • [Yes ] Liberal use is made offig.add_* andfig.update_* rather thango.Figure(data=..., layout=...) in every new/modified example
  • [ Yes] Specific adders and updaters likefig.add_shape andfig.update_xaxes are used instead of bigfig.update_layout calls in every new/modified example
  • [ Yes]fig.show() is at the end of each new/modified example
  • [ Yes]plotly.plot() andplotly.iplot() are not used in any new/modified example
  • [ Yes] Hex codes for colors are not used in any new/modified example in favour ofthese nice ones

Code PR

  • I have read through thecontributing notes and understand the structure of the package. In particular, if my PR modifies code ofplotly.graph_objects, my modifications concern thecodegen files and not generated files.
  • I have added tests (if submitting a new feature or correcting a bug) or
    modified existing tests.
  • For a new feature, I have added documentation examples in an existing or
    new tutorial notebook (please see the doc checklist as well).
  • I have added a CHANGELOG entry if fixing/changing/adding anything substantial.
  • For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).

-->

@SimaRahaSimaRaha marked this pull request as draftJanuary 26, 2025 15:57
@SimaRaha
Copy link
ContributorAuthor

Hi Greg and Liam,

I've converted this pull request to a draft as we're still working on finalizing some aspects. I'll let you know once it's ready for review.

@gvwilsongvwilson added P2considered for next cycle communitycommunity contribution documentationwritten for humans labelsFeb 3, 2025
@rl-utility-man
Copy link
Contributor

rl-utility-man commentedFeb 6, 2025
edited
Loading

This is a continuation of#4994
I rebased the git commit to make this readable and erased the history, but@SimaRaha built a complete example and had the key insight about using a secondary x axis with a separate domain.

@gvwilson
Copy link
Contributor

@LiamConnors can you please review for the Plotly.py 6.1 release?

Comment on lines +228 to +229
df = pd.read_csv('https://raw.githubusercontent.com/plotly/datasets/refs/heads/master/gss_2002_5_pt_likert.csv')
df.rename(columns={'Unnamed: 0':"Category"}, inplace=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Is there a reason to rename the column here rather in the dataset? Is that just how the dataset was?

Copy link
Contributor

@rl-utility-manrl-utility-manApr 2, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It's a shortcoming of the data set. I just proposed a PR to label the column properly in the data setplotly/datasets#64 A search of github shows no uses of that data set other than in this PR and#4994, so it appears safe to accept that PR. (I uploaded this data set recently inplotly/datasets#62 ) As soon as you mergeplotly/datasets#64 , we can remove the rename commands from this and from#4994

Comment on lines 235 to 236

fig = go.Figure()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Would it be possible to put more of the layout and data in the go.Figure object. I thinkupdate_layout and other can be a bit more challenging to understand for a user.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Sure! I moved one of the two update_layout calls into go.figure and also moved the update_legend there and the update_yaxes call there. If you do not like those, they're in atomic commits and can be rolled back. I do not see a clarifying way to eliminate the other update_layout call -- which sets up the secondary x axis and contains some variables not defined when we issue the fig = go.Figure() command. If it would be clearer to issue an update_layout that sets up both xaxis1 and xaxis2 in one call, that is straightforward. I welcome feedback about this and more specifics about other potential improvements to the structure and clarity of the code. Is there style guidance I should consult to understand this request better?

Comment on lines 221 to 222
Diverging bar charts offer two imperfect options for responses that are neither positive nor negative: put them in a separate column, as in this example or omit them as in the example above. That leaves the unreported neutral value implicit when the categories add to 100%, Jonathan Schwabish discusses this on page 92-97 of _Better Data Visualizations_.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
Diverging bar charts offer two imperfect options for responses that are neither positive nor negative: put them in a separate column, as in this example or omitthemasinthe example above. That leaves the unreported neutral value implicit when the categories add to 100%, Jonathan Schwabish discusses this on page 92-97 of_Better Data Visualizations_.
The previous diverging bar chart example excluded neutral responses. Another option, asshownin this example, is to includethem ina separate column.

I think the content around "unreported neutral values" refers the previous example? If so, I think that would be more relevant there. Also, from documentation, I'd prefer not to direct users to additional resources that they may not have access to.

Copy link
Contributor

@rl-utility-manrl-utility-manApr 16, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good point about the previous example. I moved the language there.

I think cites to work by leading thinkers on how to design effective graphs that is available from public libraries and online booksellers is a great complement to this documentation about how to implement those ideas. Also, I'd like to give Jon Schwabish's book credit for ideas that contributed to these examples. If it's better to move this cite to e.g. a comment, I'm open to that. Perhaps "Jonathan Schwabish discusses tradeoffs between these options on page 92-97 ofBetter Data Visualizations."

Comment on lines 268 to 270
legendrank=legend_rank_by_category[col],
xaxis=f"x{1+(col=="Neither Agree nor Disagree")}", # in this context, putting "Neither Agree nor Disagree" on a secondary x-axis on a different domain
# yields results equivalent to subplots with far less code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The example doesn't run for me, and I believe it's related to setting of the xaxis here.

Copy link
Contributor

@rl-utility-manrl-utility-manApr 9, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Good point. I had " where I needed ' . Fixed. Thank you!

rl-utility-man added a commit to SimaRaha/plotly.py that referenced this pull requestApr 9, 2025
@rl-utility-man
Copy link
Contributor

@LiamConnors Is there anything I can do to help finalize this? I responded to all of your points a couple weeks ago, think we're substantially in consensus on the substance and very close to done.

I pasted in a diverging bar example that's already in doc-prod; but now think it added more confusion than clarity
@gvwilson
Copy link
Contributor

@rl-utility-man thanks for all your hard work - I'll merge this as soon as Plotly.py 6.1 has gone up on PyPI (which should be today or tomorrow).

this ensures that the introductory text makes sense even if the more complex example appears first.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@LiamConnorsLiamConnorsLiamConnors left review comments

@rl-utility-manrl-utility-manrl-utility-man left review comments

Assignees
No one assigned
Labels
communitycommunity contributiondocumentationwritten for humansP2considered for next cycle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@SimaRaha@rl-utility-man@gvwilson@LiamConnors

[8]ページ先頭

©2009-2025 Movatter.jp