-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Use default colours for examples #13369
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
examples/statistics/barchart_demo.py
Outdated
student = Student('Johnny Doe', 2, 'boy') | ||
scores = dict(zip(testNames, | ||
(Score(v, p) for v, p in | ||
zip(['7', '48', '12:52', '17', '14'], | ||
np.round(np.random.uniform(0, 1, | ||
len(testNames))*100, 0))))) | ||
len(testNames)) * 100, 0))))) |
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.
the linebreak here is a bit weird
Looks like you committed an extra test2.png? Unless I'm mistaken, it should be squashed away. |
This was a lot of work, but I'm not convinced changing all the examples from standard web color names to "tab:purple" etc is very user friendly. I think I know how to use matplotlib, and even I looked at those color specifications and had to think about what the heck "tab:" meant for a couple of seconds; I can only imagine the naive user not understanding what is going on and why our colors need to be specified with a piece of magic in front of them. Whats more, I can quickly look up on wikipedia what the default web colors, I would need to spend a bit of time figuring out what the "tab:" colors are. I really apologize for this objection if this change was discussed and decided upon, but I am not convinced this is a positive change from a user's point of view. |
Don't worry, I kind of agree! In that case what I might do is put together a smaller PR where colours can be omitted altogether from the plotting, and then another PR that chooses "nice" colours (maybe black?) where they need to be explicitly specified. |
Well, maybe some of the examples stay with the "tab:x" colors (maybe with a comment explanation and link to the colors.py) and some get websafe colors and some get the short specifiers? Overall, the idea of getting the colors to look better is a great idea. I've not looked at the renders, but I imagine this looks a lot better. |
@@ -46,15 +48,7 @@ def update(val): | |||
def reset(event): | |||
sfreq.reset() | |||
samp.reset() | |||
button.on_clicked(reset) | |||
|
|||
rax = plt.axes([0.025, 0.5, 0.15, 0.15], facecolor=axcolor) |
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.
Why remove the radio buttons? Not stricly in scope but not harmful.
Pushing to 3.2 as @dstansby marked it as needs revision. If it is reading in time, re-milestone. Moving out now to reduce the clutter in the 3.1 milestone. |
ed18a6c
to
25f1a64
Compare
I'm going to close this since I don't have any plans to work on it in the future. I'll keep the branch alive though if anyone wants to take it up. |
This took longer than I thought, but I think this is all the ugly examples changed to using colours from the default colour cycle. I also made a few small changes here and there if I spotted something, but nothing major.