Opened 3 years ago

Closed 3 years ago

# Remove hardcoded default matplotlib stylesheet

Reported by: Owned by: klee major sage-8.4 graphics fbissey, strogdon Kwankyu Lee Eric Gourgoulhon, François Bissey N/A 42bda6f 42bda6f486e45c89daf5229ce89034dd5f0da0fb

This does not work:

import matplotlib as mpl
mpl.rcParams['font.family'] = 'Impact'
text("A text here", (1,1))


because of hardcoded matplotlib stylesheet (classic), which silently overwrites the rc parameters. Putting configurations in the file matplotlibrc also does not work because of the same reason.

The patch removes the hardcoded default matplotlib stylesheet that silently applies to all graphics. With this patch, the usual method to apply a style to graphics works, like

import matplotlib.pyplot as plt
plt.style.use('seaborn')
text("A text here", (1,1))


There was more discussion on this matter before at ticket:23696#comment:111.

### comment:1 Changed 3 years ago by klee

• Branch set to u/klee/25799

### comment:2 Changed 3 years ago by klee

• Description modified (diff)

### comment:3 Changed 3 years ago by git

• Commit set to 91736728f417dd7598a0eff1ec30bf6125a6fa2a

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

 ​9173672 Remove hardcoded matplotlib stylesheet

### comment:4 Changed 3 years ago by klee

• Status changed from new to needs_review

### comment:5 Changed 3 years ago by klee

• Description modified (diff)

### comment:6 Changed 3 years ago by egourgoulhon

For reference, I am copying below a part of this sage-devel thread:

we may keep specifically only the math font of the classic style by:

mpl.rcParams['mathtext.fontset'] = 'cm'
mpl.rcParams['mathtext.rm'] = 'serif'


See this example for drawbacks of using Matplolib new defaults for math fonts. Moreover using cm fonts for math makes plots coherent with MathJax rendering of Jupyter output cells, which is an important feature IMHO.

### comment:7 Changed 3 years ago by git

• Commit changed from 91736728f417dd7598a0eff1ec30bf6125a6fa2a to 7828ce2440ed53505c8c6e10e6a16effe1c2e4b5

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

 ​7828ce2 Set computer modern fonts for math text in graphics

### comment:8 Changed 3 years ago by klee

• Description modified (diff)

### comment:9 Changed 3 years ago by egourgoulhon

• Priority changed from minor to major

Changing the priority to 'major' in order to get the attention of patchbots.

### comment:10 follow-up: ↓ 12 Changed 3 years ago by egourgoulhon

A noticeable effect of the proposed change is that it decreases the size of figures of approx. 20% in each dimension. For instance, with the ticket branch,

sage: plot(sin(x^2), (x, 0, 4), axes_labels=['$x$', '$y$'], gridlines=True)


generates this 628x470 figure:

while we had previously (i.e. with the 'classic' style) this 781x584 figure:

This size change was already discussed in comment:134 of #23696.

Other changes are the major ticks on the x-axis (Delta_x = 0.5 instead of Delta_x = 1) and the dash style (in general, dashes are narrower than with the classic style, as we can see on the dash plots in the 2D graphics section of the documentation).

I am fine with these changes (*); I simply mention them for the benefit of the discussion.

(*) except maybe for the dash style of curves, which could require some tuning of the option linestyle='--' on Sage's side: compare the output of

sage: g1 = plot(sin(x), 0, 2*pi)
sage: g2 = plot(cos(x), 0, 2*pi, linestyle="--")
sage: (g1+g2).show(ticks=pi/6, tick_formatter=pi)  # long time # show their sum, nicely formatted


in the 2D plotting section of the documentation.

On general grounds, I think it is a good thing to remove the 'classic' stylesheet (provided the TeX fonts are corrected as above), in particular because this fixes the issues mentioned in the ticket description.

### comment:12 in reply to: ↑ 10 Changed 3 years ago by klee

I am fine with these changes (*); I simply mention them for the benefit of the discussion.

(*) except maybe for the dash style of curves, which could require some tuning of the option linestyle='--' on Sage's side: compare the output of

sage: g1 = plot(sin(x), 0, 2*pi)
sage: g2 = plot(cos(x), 0, 2*pi, linestyle="--")
sage: (g1+g2).show(ticks=pi/6, tick_formatter=pi)  # long time # show their sum, nicely formatted


in the 2D plotting section of the documentation.

I am not sure...

Dashed line in classic style:

Dashed line in matplotlib2 default style:

### comment:13 follow-up: ↓ 14 Changed 3 years ago by fbissey

While the font change was the most glaring thing, there was other stuff indeed. Changing the style was the shortest way to fix all that. It would be interesting to know if we can set a style just for building the documentation. Of course even that could cause problems if you need to change something for building the documentation in a specific language.

### comment:14 in reply to: ↑ 13 Changed 3 years ago by klee

While the font change was the most glaring thing, there was other stuff indeed. Changing the style was the shortest way to fix all that.

If there are needed other (not uniformly agreed upon) tunings, then it may be necessary to make another ticket for that purpose, while the most glaring changes, like the font change, get into this ticket. Let's see.

It would be interesting to know if we can set a style just for building the documentation. Of course even that could cause problems if you need to change something for building the documentation in a specific language.

Yes. But as someone has suggested in other context, I agree that users should get the same outputs in their own sessions as in the documentation.

### comment:15 Changed 3 years ago by klee

Just for discussion.

Dashed linesin classic style:

Dashed lines in default style:

Version 0, edited 3 years ago by klee (next)

### comment:16 Changed 3 years ago by klee

There was a poll about the changes that the branch of this ticket introduces in

Counted from 8 participants to the discussion, there are

2 no opinion;

1 not-so-positive vote.

### comment:17 Changed 3 years ago by egourgoulhon

• Milestone changed from sage-8.3 to sage-8.4
• Reviewers set to Eric Gourgoulhon
• Status changed from needs_review to positive_review

So let's move this into Sage 8.4!

### comment:18 Changed 3 years ago by egourgoulhon

• Authors set to Kwankyu Lee

### comment:19 follow-up: ↓ 20 Changed 3 years ago by vbraun

• Status changed from positive_review to needs_work
sage -t --long src/sage/plot/arrow.py  # 1 doctest failed


### comment:20 in reply to: ↑ 19 ; follow-up: ↓ 21 Changed 3 years ago by fbissey

sage -t --long src/sage/plot/arrow.py  # 1 doctest failed


That doctest is fragile and I am pretty sure it depends on the stylesheet used.

### comment:21 in reply to: ↑ 20 Changed 3 years ago by egourgoulhon

sage -t --long src/sage/plot/arrow.py  # 1 doctest failed


That doctest is fragile and I am pretty sure it depends on the stylesheet used.

I confirm the doctest fails with the ticket branch merged in Sage 8.3.rc0:

./sage -t --long src/sage/plot/arrow.py
Git branch: matplotlib_style
Using --optional=mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 80.1 src/sage/plot/arrow.py
**********************************************************************
File "src/sage/plot/arrow.py", line 364, in sage.plot.arrow.Arrow._render_on_subplot
Failed example:
two_stroke_re.search(contents) is None
Expected:
True
Got:
False
**********************************************************************
1 of  18 in sage.plot.arrow.Arrow._render_on_subplot
[69 tests, 1 failure, 3.79 s]


### comment:22 follow-up: ↓ 23 Changed 3 years ago by fbissey

Getting that doctest to return something sensible required a considerable amount of effort. It is about controlling some aspect of the appearance on screen and stems from an older ticket about the appearance of double dashing. If I am completely honest I am not sure it is worth the time invested into fixing it.

### comment:23 in reply to: ↑ 22 ; follow-up: ↓ 28 Changed 3 years ago by klee

Getting that doctest to return something sensible required a considerable amount of effort. It is about controlling some aspect of the appearance on screen and stems from an older ticket about the appearance of double dashing. If I am completely honest I am not sure it is worth the time invested into fixing it.

I agree. Doctesting graphics is something that requires a human eye or AI.

### comment:24 Changed 3 years ago by git

• Commit changed from 7828ce2440ed53505c8c6e10e6a16effe1c2e4b5 to 42bda6f486e45c89daf5229ce89034dd5f0da0fb

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

 ​a825afb Remove a fragile failing doctest ​42bda6f Add a comment on the removed doctest

### comment:25 Changed 3 years ago by klee

• Status changed from needs_work to needs_review

### comment:26 Changed 3 years ago by egourgoulhon

• Reviewers changed from Eric Gourgoulhon to Eric Gourgoulhon, François Bissey
• Status changed from needs_review to positive_review

LGTM. François, do you agree?

### comment:27 Changed 3 years ago by fbissey

I am OK with it. I'll be waiting for comments on the new look of the documentation. It is inevitable, even when we have talked so much about it.

### comment:28 in reply to: ↑ 23 ; follow-up: ↓ 29 Changed 3 years ago by strogdon

Getting that doctest to return something sensible required a considerable amount of effort. It is about controlling some aspect of the appearance on screen and stems from an older ticket about the appearance of double dashing. If I am completely honest I am not sure it is worth the time invested into fixing it.

I agree. Doctesting graphics is something that requires a human eye or AI.

It appears that now we are back to the matching sequence for 1.5.3 https://trac.sagemath.org/ticket/23696#comment:27. This

sage: two_stroke_pattern = r'setdash.*setdash.*stroke.*stroke.*setdash'


seems to work; although, I haven't checked whether the above is matched correctly when the arrow is drawn with dashed linetype.

### comment:29 in reply to: ↑ 28 Changed 3 years ago by strogdon

Getting that doctest to return something sensible required a considerable amount of effort. It is about controlling some aspect of the appearance on screen and stems from an older ticket about the appearance of double dashing. If I am completely honest I am not sure it is worth the time invested into fixing it.

I agree. Doctesting graphics is something that requires a human eye or AI.

It appears that now we are back to the matching sequence for 1.5.3 https://trac.sagemath.org/ticket/23696#comment:27. This

sage: two_stroke_pattern = r'setdash.*setdash.*stroke.*stroke.*setdash'


seems to work; although, I haven't checked whether the above is matched correctly when the arrow is drawn with dashed linetype.

This was just a shot in the dark. It doesn't work properly when the arrow is drawn with dashed linetype.

### comment:30 Changed 3 years ago by strogdon

Just for the record, not that anything should be changed but

sage: two_stroke_pattern = r'setdash.*setdash.*stroke.*stroke.*setdash'


is the correct matching pattern. It's fairly straightforward to deduce it. I had a typo that led to the previous conclusion.

### comment:31 Changed 3 years ago by vbraun

• Branch changed from u/klee/25799 to 42bda6f486e45c89daf5229ce89034dd5f0da0fb
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.