Opened 2 years ago

Closed 23 months ago

#19485 closed enhancement (fixed)

Legible legends

Reported by: mjo Owned by:
Priority: major Milestone: sage-6.10
Component: graphics Keywords: beginner
Cc: Merged in:
Authors: Michael Orlitzky Reviewers: Jori Mäntysalo, Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: 8aa1ef5 (Commits) Commit: 8aa1ef5188f67a9fcc906b21f3b98e362a804ba7
Dependencies: Stopgaps:

Description

The default "legend options" are empty, but this defaults everywhere to a gray legend with black text. This is difficult to read, especially when the result is printed (journal, thesis, etc.) or projected (as in a presentation).

Black-on-white is much better, and a drop shadow helps the legend stand out without affecting the text within.

Attachments (2)

plot-old.png (18.0 KB) - added by mjo 2 years ago.
Example legend (current defaults)
plot-new.png (18.1 KB) - added by mjo 2 years ago.
Example legend (new defaults)

Download all attachments as: .zip

Change History (20)

Changed 2 years ago by mjo

Example legend (current defaults)

comment:1 Changed 2 years ago by mjo

  • Authors set to Michael Orlitzky
  • Branch set to u/mjo/ticket/19485
  • Commit set to 2148f20bedec3f4c7bac7f056bf5065b13f3ba8d
  • Status changed from new to needs_review

New commits:

2148f20Trac #19485: make legends more legible.

Changed 2 years ago by mjo

Example legend (new defaults)

comment:2 follow-up: Changed 2 years ago by kcrisman

I don't really care that much, but I do think that the drop-shadow maybe shouldn't be a default. At least according to a lot of design stuff I read at some point about the equivalent CSS thing in web pages. Do you think a much lighter gray would work better? Maybe the white might not distinguish enough between the stuff and the legend (sort of a random box).

But really I think that the matplotlib defaults might be the wisest to check, or see what competitor X does for some value(s) of X. Then it's not so much bikeshedding as seeing what standard (if not to say best, since it might not be best!) practice is. I'd be reluctant to change it without at least a few people who use this fairly regularly saying changing the default is a good idea - otherwise you can just change YOUR defaults, if I recall correctly.

(Thanks for including the pics, by the way.)

comment:3 in reply to: ↑ 2 Changed 2 years ago by mjo

Replying to kcrisman:

I don't really care that much, but I do think that the drop-shadow maybe shouldn't be a default.

It feels a little weird "enabling" something by default, but what it came down to for me is that it just looks better with the drop shadow than without. I realized that I was thinking about making it look worse just so a bit could be turned off.

At least according to a lot of design stuff I read at some point about the equivalent CSS thing in web pages.

Creating drop shadows with CSS was a nightmare for other reasons. The standard way didn't work, and each browser had its own "beta" implementation (none of which agreed), and as usual, nothing worked in MSIE.

I do understand that adding bling bling should be done with care, but hopefully people agree that the drop shadow is unobtrusive and creates a nice contrast.

Do you think a much lighter gray would work better? Maybe the white might not distinguish enough between the stuff and the legend (sort of a random box).

Yeah, that's why I enabled the drop shadow by default. Anything darker than white makes the text harder to read, and "light gray" prints terribly.

otherwise you can just change YOUR defaults, if I recall correctly.

It's really hard! But that argument works against me if somebody really likes the gray. This was the best I could come up with:

plot_impl = sage.plot.plot._plot

def mjo_plot(*args, **kwargs):
    """
    Replacement for the default plot function.

     - If there's a legend, set the background color to 'white' and
       give it a drop shadow.

    """
    default_legend_opts = { 'back_color': 'white',
                            'shadow': True }

    p = plot_impl(*args, **kwargs)
    p.set_legend_options(**default_legend_opts)

    return p

sage.plot.plot._plot = mjo_plot

You need to do the same for list_plot and all of the graphics primitives, too.

Last edited 2 years ago by mjo (previous) (diff)

comment:4 Changed 2 years ago by kcrisman

Oh, interesting; I figured that sage.plot.plot.DEFAULT_OPTIONS or some such dictionary would be a sufficient incantation in a init.sage file. That certainly is a problem, in that event.

comment:5 Changed 2 years ago by jmantysalo

I like this suggestion. It seems better than current default.

comment:6 Changed 23 months ago by jmantysalo

  • Reviewers set to Jori Mäntysalo

Passed tests, and the code seems to have no errors.

comment:7 Changed 23 months ago by jmantysalo

  • Status changed from needs_review to positive_review

comment:8 follow-up: Changed 23 months ago by kcrisman

  • Status changed from positive_review to needs_info

I would suggest at least putting how to get the previous behavior back for those who would want it.

comment:9 in reply to: ↑ 8 Changed 23 months ago by mjo

Replying to kcrisman:

I would suggest at least putting how to get the previous behavior back for those who would want it.

Put it where?

The old settings were equivalent to,

sage: p = plot(sin(x), legend_label='$\sin(x)$')
sage: p.set_legend_options(back_color=(0.9,0.9,0.9), shadow=False) # option 1
sage: p.show(legend_options={'back_color': (0.9,0.9,0.9), 'shadow': False}) # option 2

but there's no easy way to change the behavior wholesale. In comment:3 I showed how I was able to do it for plot, but you would also have to do it for list_plot, line, circle, etc.

comment:10 Changed 23 months ago by kcrisman

I think that is pretty good as an example to put in the main plot documentation and the show documentation, something like "if you want a white background without shadow, do this..."

comment:11 Changed 23 months ago by git

  • Commit changed from 2148f20bedec3f4c7bac7f056bf5065b13f3ba8d to 8aa1ef5188f67a9fcc906b21f3b98e362a804ba7

Branch pushed to git repo; I updated commit sha1. New commits:

8aa1ef5Trac #19485: Add examples to plot? and show? of the old legend appearance.

comment:12 Changed 23 months ago by mjo

  • Status changed from needs_info to needs_review

Do you think that's sufficient?

I would like it if this were easier to change globally, but given that it isn't, I do think this new style is an improvement.

comment:13 follow-up: Changed 23 months ago by jmantysalo

Very minor thing, but I would like more "Prior to Sage version 6.10 - -" instead of trac number.

comment:14 in reply to: ↑ 13 ; follow-up: Changed 23 months ago by mjo

Replying to jmantysalo:

Very minor thing, but I would like more "Prior to Sage version 6.10 - -" instead of trac number.

Can you get Volker to promise to merge it for 6.10? =)

comment:15 in reply to: ↑ 14 Changed 23 months ago by jmantysalo

Replying to mjo:

Replying to jmantysalo:

Very minor thing, but I would like more "Prior to Sage version 6.10 - -" instead of trac number.

Can you get Volker to promise to merge it for 6.10? =)

At least we have plenty of time for that, as we are now at beta 3.

As the patch: For me this could be positive_review, but let kcrisman to say if the documentation is good enought.

comment:16 follow-up: Changed 23 months ago by kcrisman

I'm fine with it. Don't complain if people find the new behavior too vexing :)

comment:17 in reply to: ↑ 16 Changed 23 months ago by jmantysalo

  • Reviewers changed from Jori Mäntysalo to Jori Mäntysalo, Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

Replying to kcrisman:

I'm fine with it. Don't complain if people find the new behavior too vexing :)

So let's mark this as positive review.

comment:18 Changed 23 months ago by vbraun

  • Branch changed from u/mjo/ticket/19485 to 8aa1ef5188f67a9fcc906b21f3b98e362a804ba7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.