Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
DOC: (subjectively) nicer annotated barchart example#9907
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I've merged because I would be very surpised if anyone thought this version was not an improvement over the last! Thanks! |
@jklymak Well sometimes there is a reasonable argument about not changing some of the aesthetic part of the code as it is usually a matter of taste and could lead to a pile of small value commits (IIUC). And I know that some other devs lean more toward the idea that one example should focus one feature and minimize cosmetick tweaks, in order to make the message clearer. I kind of agree with the general idea but sometimes it leads to quite unpleasant plots on purely aesthetic grounds (due to the historical defaults usually), which I also find sad as those example are often one of the main sources of ideas for end-users. |
Sure, and if your code did a bunch of obfuscated or wacky things, I wouldn't have merged it (though I was probably bad to not allow PEP8 to finish - ahem). |
FWIW, I checked it locally with |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Here is a PR that:
ind - width/2
andind + width/2
instead ofind
andind + width
, etc.)I am open to revert part or all of 2. and 3. due to their subjective nature. However, I really think that 1. would help to make this example more attractive for people looking for a way to “simply” annotate bar charts (which seems rather frequent by wandering on the Internet) that is still quite pleasant to the eye.
Original

Suggestion

PR Checklist