FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

pie_chart_with_horizontal_bar_chart added · Pull Request #2578 · matplotlib/matplotlib · GitHub

pie_chart_with_horizontal_bar_chart added - #2578

Closed
ghost wants to merge 1 commit into
masterfrom
unknown repository
Closed

ghost wants to merge 1 commit into
masterfrom
unknown repository

Conversation

ghost commented Nov 5, 2013

Copy link
Copy Markdown

Simple Bar chart with leading horizontal bar chart added to examples/pie_and_polar_charts section.

Copy link
Copy Markdown
Member

Can you also add this to the documentation someplace (gallery?)

Please see https://github.com/matplotlib/matplotlib/wiki/MEP12, #2181 and #2474

tonysyu commented Nov 8, 2013

Copy link
Copy Markdown
Contributor

I hope this doesn't come off the wrong way, but I don't really understand the point of this example. There are examples for bar charts and pie charts that demonstrate the functionality here, and I don't think combining the two into one plot warrants a new example.

Sorry to be so negative, but I'm of the opinion that the gallery needs to be trimmed down.

efiring commented Nov 8, 2013

Copy link
Copy Markdown
Member

I agree with @tonysu on this.

ghost commented Nov 8, 2013

Copy link
Copy Markdown
Author

The idea behind this example was to add more example to the Library. In my view more example we have the more useful the Library is.
@tonysyu is right with his point of view. The example does not add any new functionality but serve as an another use of this two charts types.

ghost commented Nov 8, 2013

Copy link
Copy Markdown
Author

After reviewing the MEP12 (which @tacaswell suggested) more carefully, not to add this new example to the Library may be more reasonable to keep example sections simple and not to repeat ourselves.
In my opinion, having more and more example is quite important for useful library. On the other hand, examples should be organized carefully for usability and simplicity. ( @tonysyu is right in his point)

So, not to merge this patch will be the best.

ghost closed this Nov 8, 2013
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL